Re: [PATCH v15 4/7] drm/bridge: dw-hdmi: repair interworking with hdmi-connector for jz4780
Hi, On 12/02/2022 16:50, H. Nikolaus Schaller wrote: Commit 7cd70656d1285b ("drm/bridge: display-connector: implement bus fmts callbacks") introduced a new mechanism to negotiate bus formats between hdmi connector and the synopsys hdmi driver inside the jz4780. By this, the dw-hdmi is no longer the only bridge and sets up a list of formats in dw_hdmi_bridge_atomic_get_output_bus_fmts(). This includes MEDIA_BUS_FMT_UYVY8_1X16 which is chosen for the jz4780 but only produces a black screen. This fix is based on the observation that max_bpc = 0 when running this function while info->bpc = 8. Since the formats checks before this always test for max_bpc >= info->pbc indirectly my assumption is that we must check it here as well. Adding the proposed patch makes the CI20/jz4780 panel work again in MEDIA_BUS_FMT_RGB888_1X24 mode. Fixes: 7cd70656d1285b ("drm/bridge: display-connector: implement bus fmts callbacks") Signed-off-by: H. Nikolaus Schaller --- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index b0d8110dd412c..826a055a7a273 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -2620,10 +2620,10 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge, output_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30; } - if (info->color_formats & DRM_COLOR_FORMAT_YCBCR422) + if (max_bpc >= info->bpc && info->color_formats & DRM_COLOR_FORMAT_YCBCR422) output_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16; - if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444) + if (max_bpc >= info->bpc && info->color_formats & DRM_COLOR_FORMAT_YCBCR444) output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24; /* Default 8bit RGB fallback */ Please do the same for all other cases and change the patch subject to something more accurate like: "drm/bridge: dw-hdmi: take display info bpc in account for bus formats negociation" Neil
Re: [PATCH v16 21/40] pwm: tegra: Add runtime PM and OPP support
Hello, On Wed, Dec 01, 2021 at 02:23:28AM +0300, Dmitry Osipenko wrote: > The PWM on Tegra belongs to the core power domain and we're going to > enable GENPD support for the core domain. Now PWM must be resumed using > runtime PM API in order to initialize the PWM power state. The PWM clock > rate must be changed using OPP API that will reconfigure the power domain > performance state in accordance to the rate. Add runtime PM and OPP > support to the PWM driver. > > Reviewed-by: Ulf Hansson > Signed-off-by: Dmitry Osipenko > --- > drivers/pwm/pwm-tegra.c | 82 - > 1 file changed, 64 insertions(+), 18 deletions(-) > > diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c > index 11a10b575ace..18cf974ac776 100644 > --- a/drivers/pwm/pwm-tegra.c > +++ b/drivers/pwm/pwm-tegra.c > @@ -42,12 +42,16 @@ > #include > #include > #include > +#include > #include > #include > #include > +#include > #include > #include > > +#include > + > #define PWM_ENABLE (1 << 31) > #define PWM_DUTY_WIDTH 8 > #define PWM_DUTY_SHIFT 16 > @@ -145,7 +149,7 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct > pwm_device *pwm, > required_clk_rate = > (NSEC_PER_SEC / period_ns) << PWM_DUTY_WIDTH; > > - err = clk_set_rate(pc->clk, required_clk_rate); > + err = dev_pm_opp_set_rate(pc->dev, required_clk_rate); > if (err < 0) > return -EINVAL; > > @@ -181,8 +185,8 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct > pwm_device *pwm, >* before writing the register. Otherwise, keep it enabled. >*/ > if (!pwm_is_enabled(pwm)) { > - err = clk_prepare_enable(pc->clk); > - if (err < 0) > + err = pm_runtime_resume_and_get(pc->dev); > + if (err) > return err; > } else > val |= PWM_ENABLE; > @@ -193,7 +197,7 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct > pwm_device *pwm, >* If the PWM is not enabled, turn the clock off again to save power. >*/ > if (!pwm_is_enabled(pwm)) > - clk_disable_unprepare(pc->clk); > + pm_runtime_put(pc->dev); > > return 0; > } > @@ -204,8 +208,8 @@ static int tegra_pwm_enable(struct pwm_chip *chip, struct > pwm_device *pwm) > int rc = 0; > u32 val; > > - rc = clk_prepare_enable(pc->clk); > - if (rc < 0) > + rc = pm_runtime_resume_and_get(pc->dev); > + if (rc) > return rc; > > val = pwm_readl(pc, pwm->hwpwm); > @@ -224,7 +228,7 @@ static void tegra_pwm_disable(struct pwm_chip *chip, > struct pwm_device *pwm) > val &= ~PWM_ENABLE; > pwm_writel(pc, pwm->hwpwm, val); > > - clk_disable_unprepare(pc->clk); > + pm_runtime_put_sync(pc->dev); > } > > static const struct pwm_ops tegra_pwm_ops = { > @@ -256,11 +260,20 @@ static int tegra_pwm_probe(struct platform_device *pdev) > if (IS_ERR(pwm->clk)) > return PTR_ERR(pwm->clk); > > + ret = devm_tegra_core_dev_init_opp_table_common(&pdev->dev); > + if (ret) > + return ret; > + > + pm_runtime_enable(&pdev->dev); > + ret = pm_runtime_resume_and_get(&pdev->dev); > + if (ret) > + return ret; > + > /* Set maximum frequency of the IP */ > - ret = clk_set_rate(pwm->clk, pwm->soc->max_frequency); > + ret = dev_pm_opp_set_rate(pwm->dev, pwm->soc->max_frequency); > if (ret < 0) { > dev_err(&pdev->dev, "Failed to set max frequency: %d\n", ret); > - return ret; > + goto put_pm; > } > > /* > @@ -278,7 +291,7 @@ static int tegra_pwm_probe(struct platform_device *pdev) > if (IS_ERR(pwm->rst)) { > ret = PTR_ERR(pwm->rst); > dev_err(&pdev->dev, "Reset control is not found: %d\n", ret); > - return ret; > + goto put_pm; > } > > reset_control_deassert(pwm->rst); > @@ -291,10 +304,16 @@ static int tegra_pwm_probe(struct platform_device *pdev) > if (ret < 0) { > dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret); > reset_control_assert(pwm->rst); > - return ret; > + goto put_pm; > } > > + pm_runtime_put(&pdev->dev); > + > return 0; > +put_pm: > + pm_runtime_put_sync_suspend(&pdev->dev); > + pm_runtime_force_suspend(&pdev->dev); > + return ret; > } > > static int tegra_pwm_remove(struct platform_device *pdev) > @@ -305,20 +324,44 @@ static int tegra_pwm_remove(struct platform_device > *pdev) > > reset_control_assert(pc->rst); > > + pm_runtime_force_suspend(&pdev->dev); > + > return 0; > } > > -#ifdef CONFIG_PM_SLEEP > -static int tegra_pwm_suspend(struct device *dev) > +static int __maybe_unused tegra_pwm_runtime_susp
Re: [PATCH] drm/amdgpu: check vm bo eviction valuable at last
Am 21.02.22 um 04:28 schrieb Qiang Yu: On Fri, Feb 18, 2022 at 6:24 PM Christian König wrote: Am 18.02.22 um 11:16 schrieb Qiang Yu: [SNIP] If amdgpu_vm_ready() use evicting flag, it's still not equivalent to check vm idle: true -> vm idle, false -> vm may be idle or busy. Yeah, but why should that be relevant? The amdgpu_vm_ready() return if we can do page table updates or not. If the VM is idle or not is only relevant for eviction. In other words any CS or page table update makes the VM busy, but that only affects if the VM can be evicted or not. My point is: we can't use amdgpu_vm_ready() to replace vm_is_busy(), so currently we update vm even when vm is busy. So why not use: Sorry, should be "vm is idle". if (!amdgpu_vm_ready() || vm_is_busy()) return; in amdgpu_gem_va_update_vm(), as you mentioned we prefer to not update vm when it's idle. Because updating the VM while it is busy is perfectly fine, we do it all the time. Yeah, as above, my typo. We should just not update it when it is already idle and was considered for eviction. "and", not "or"? In this situation it makes most of the time sense to keep it idle and postpone the update till the next command submission. Then we can keep the evicting flag accurate (after solving your concern for this patch that eviction may fail latter by further delay the flag update after eviction success). That won't work. See we need to mark the VM as evicted before we actually evict them because otherwise somebody could use the VM in parallel and add another fence to it. I see, make this too accurate should cost too much like holding the eviction_lock when eviction. But just delay it in amdgpu_ttm_bo_eviction_valuable() could avoid most false positive case. Partially correct. Another fundamental problem is that we can't hold the eviction lock because that would result in lock inversion and potential deadlock. We could set the flag later on, but as I said before that when we set the evicted flag when the VM is already idle is a desired effect. As above, this confuse me as we can explicitly check vm idle when user update vm, why bother to embed it in evicting flag implicitly? Well as I said it's irrelevant for the update if the VM is idle or not. To summarize the rules once more: 1. When VM page tables are used by CS or page tables updates it is considered busy, e.g. not idle. 2. When we want to evict a VM it must be idle. As soon as we considered this we should set the evicted flag to make sure to keep it idle as much as possible. 3. When we want to update the page tables we just need to check if the VM is idle or not. But now we does not check vm idle directly in amdgpu_gem_va_update_vm(). If VM bo has not been considered for eviction, it could be either idle or busy. Just want to confirm if the fix should be only change amdgpu_vm_ready() to use evicting flag or besides using evicting flag, also check vm_idle() in amdgpu_gem_va_update_vm(). Only changing the amdgpu_vm_ready() should be enough. It can be that this then bubbles up more issue, but those need to be taken care of separately then. Regards, Christian. Regards, Qiang 4. When a CS happens we don't have another chance and make the VM busy again. And do all postponed page table updates. Anyway, Regards, Christian. Check vm idle need to hold resv lock. Read your patch for adding evicting flag is to update vm without resv lock. But user vm ops in amdgpu_gem_va_update_vm() do hold the resv lock, so the difference happens when calling amdgpu_vm_bo_update_mapping() from svm_range_(un)map_to_gpu(). So embed vm idle in evicting flag is for svm_range_(un)map_to_gpu() also do nothing when vm idle? Regards, Qiang Regards, Christian. Regards, Qiang Regards, Christian. Regards, Qiang Regards, Christian. Regards, Qiang Regards, Christian. Regards, Qiang Regards, Christian. Regards, Qiang What we should rather do is to fix amdgpu_vm_ready() to take a look at the flag instead of the linked list. Regards, Christian. Signed-off-by: Qiang Yu --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 85 ++--- 1 file changed, 47 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 5a32ee66d8c8..88a27911054f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1306,45 +1306,11 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct amdgpu_device *adev, struct ttm_tt *ttm, return flags; } -/* - * amdgpu_ttm_bo_eviction_valuable - Check to see if we can evict a buffer - * object. - * - * Return true if eviction is sensible. Called by ttm_mem_evict_first() on - * behalf of ttm_bo_mem_force_space() which tries to evict buffer objects until - * it can find space for a new object and by ttm_bo_force_list_clean() which is - * used to clean out a memory space. - */ -static bool amdgpu_ttm_b
Re: Regression from 3c196f056666 ("drm/amdgpu: always reset the asic in suspend (v2)") on suspend?
On 20/02/2022 16:48, Dominique Dumont wrote: On Monday, 14 February 2022 22:52:27 CET Alex Deucher wrote: Does the system actually suspend? Not really. The screens looks like it's going to suspend, but it does come back after 10s or so. The light mounted in the middle of the power button does not switch off. As I have a very similar problem and also commented on the original debian bug report (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1005005), I will add some information here on another amd only laptop (renoir AMD Ryzen 7 4800H with Radeon Graphics + Radeon RX 5500/5500M / Pro 5500M). For me the suspend works once, but after the first resume (I do know know if it is in the suspend path or the resume path I see a RIP in the dmesg (see aditional info in debian bug)) and later suspend do not work: It only go to the kde login screen. I was unable due to network connectivity to do a full bisect but tested with the patch I had on my laptop: 5.10.101 works, 5.10 from debian works 5.11 works 5.12 works 5.13 suspend works but when resuming the PC is dead I have to reboot 5.14 seems to work but looking at dmesg it is full of RIP messages at various places. 5.15.24 is a described 5.15 from debian is behaving identically 5.16 from debian is behaving identically. Is this system S0i3 or regular S3? For me it is real S3. The proposed patch is intended for INTEl + intel gpu + amdgpu but I have dual amd GPU. --eric
Re: Regression from 3c196f056666 ("drm/amdgpu: always reset the asic in suspend (v2)") on suspend?
On Monday, 14 February 2022 22:52:27 CET Alex Deucher wrote: > Does the system actually suspend? Not really. The screens looks like it's going to suspend, but it does come back after 10s or so. The light mounted in the middle of the power button does not switch off. > Is this system S0i3 or regular S3? I'm not sure how to check that. After a bit of reading on the Internet [1], I hope that the following information answers that question. Please get back to me if that's not the case. Looks like my system supports both Soi3 and S3 $ cat /sys/power/state freeze mem disk I get the same result running these 2 commands as root: # echo freeze > /sys/power/state # echo mem > /sys/power/state > Does this patch help by any chance? > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i > d=e55a3aea418269266d84f426b3bd70794d3389c8 yes, with this patch: - the suspend issue is solved - kernel logs no longer show messages like "failed to send message" or "*ERROR* suspend of IP block failed" while suspending All the best [1] https://01.org/blogs/rzhang/2015/best-practice-debug-linux-suspend/ hibernate-issues
Re: [PATCH V2] drm/imx: parallel-display: Remove bus flags check in imx_pd_bridge_atomic_check()
Hello Christoph, On Sat, 19 Feb 2022 09:28:44 + Christoph Niedermaier wrote: > From: Max Krummenacher [mailto:max.oss...@gmail.com] > Sent: Wednesday, February 9, 2022 10:38 AM > >> If display timings were read from the devicetree using > >> of_get_display_timing() and pixelclk-active is defined > >> there, the flag DISPLAY_FLAGS_SYNC_POSEDGE/NEGEDGE is > >> automatically generated. Through the function > >> drm_bus_flags_from_videomode() e.g. called in the > >> panel-simple driver this flag got into the bus flags, > >> but then in imx_pd_bridge_atomic_check() the bus flag > >> check failed and will not initialize the display. The > >> original commit fe141cedc433 does not explain why this > >> check was introduced. So remove the bus flags check, > >> because it stops the initialization of the display with > >> valid bus flags. > >> > >> Fixes: fe141cedc433 ("drm/imx: pd: Use bus format/flags provided by the > >> bridge when available") > >> Signed-off-by: Christoph Niedermaier > >> Cc: Marek Vasut > >> Cc: Boris Brezillon > >> Cc: Philipp Zabel > >> Cc: David Airlie > >> Cc: Daniel Vetter > >> Cc: Shawn Guo > >> Cc: Sascha Hauer > >> Cc: Pengutronix Kernel Team > >> Cc: Fabio Estevam > >> Cc: NXP Linux Team > >> Cc: linux-arm-ker...@lists.infradead.org > >> To: dri-devel@lists.freedesktop.org > >> --- > >> V2: - Add Boris to the Cc list > >> --- > >> drivers/gpu/drm/imx/parallel-display.c | 8 > >> 1 file changed, 8 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/imx/parallel-display.c > >> b/drivers/gpu/drm/imx/parallel-display.c > >> index a8aba0141ce7..06cb1a59b9bc 100644 > >> --- a/drivers/gpu/drm/imx/parallel-display.c > >> +++ b/drivers/gpu/drm/imx/parallel-display.c > >> @@ -217,14 +217,6 @@ static int imx_pd_bridge_atomic_check(struct > >> drm_bridge *bridge, > >> if (!imx_pd_format_supported(bus_fmt)) > >> return -EINVAL; > >> > >> - if (bus_flags & > >> - ~(DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_DE_HIGH | > >> - DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE | > >> - DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)) { > >> - dev_warn(imxpd->dev, "invalid bus_flags (%x)\n", bus_flags); > >> - return -EINVAL; > >> - } > >> - > >> bridge_state->output_bus_cfg.flags = bus_flags; > >> bridge_state->input_bus_cfg.flags = bus_flags; > >> imx_crtc_state->bus_flags = bus_flags; > > > > Tested on a Colibri iMX6DL with a panel-dpi based panel. > > > > Tested-by: Max Krummenacher > > I still ask myself why this bus flag check is in the code. > Is there a reason not to remove that? The reasoning was that DE_{LOW,HIGH} and FLAG_PIXDATA_DRIVE_{POS,NEG}EDGE were the only bus_flags taken into account by the crtc logic, so anything else is simply ignored. This was definitely wrong since the driver supports at least one of the VSYNC polarity (perhaps both if there's a way to configure it that's not hooked-up yet). So I guess figuring out the default VSYNC polarity and accepting the according DISPLAY_FLAGS_SYNC_XXXEDGE flag is what makes most sense here. Regards, Boris
Aw: [PATCH v6 17/23] arm64: dts: rockchip: rk356x: Add HDMI nodes
Hi > Gesendet: Donnerstag, 17. Februar 2022 um 09:29 Uhr > Von: "Sascha Hauer" > --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi > @@ -10,7 +10,6 @@ > #include > #include > #include > -#include why dropping this after adding in part 16? > #include it looks like you moved this to board includes...imho this should stay in the rk356x.dtsi, because compilation will fail if a board without the vop2 (and missing the include) is derived from rk356x.dtsi. regards Frank
Re: [PATCH] drm/bridge: anx7625: switch to devm_drm_of_get_bridge
On Mon, Feb 21, 2022 at 08:28:35AM +0100, José Expósito wrote: > The function "drm_of_find_panel_or_bridge" has been deprecated in > favor of "devm_drm_of_get_bridge". > > Switch to the new function and reduce boilerplate. > > Signed-off-by: José Expósito Reviewed-by: Maxime Ripard Maxime signature.asc Description: PGP signature
Re: [PATCH] drm/msm/dp: switch to devm_drm_of_get_bridge
On Mon, Feb 21, 2022 at 08:33:39AM +0100, José Expósito wrote: > The function "drm_of_find_panel_or_bridge" has been deprecated in > favor of "devm_drm_of_get_bridge". > > Switch to the new function and reduce boilerplate. > > Signed-off-by: José Expósito Reviewed-by: Maxime Ripard Maxime signature.asc Description: PGP signature
Re: [PATCH] drm: rcar-du: switch to devm_drm_of_get_bridge
On Mon, Feb 21, 2022 at 08:37:57AM +0100, José Expósito wrote: > The function "drm_of_find_panel_or_bridge" has been deprecated in > favor of "devm_drm_of_get_bridge". > > Switch to the new function and reduce boilerplate. > > Signed-off-by: José Expósito > --- > drivers/gpu/drm/rcar-du/rcar_lvds.c | 16 +--- > 1 file changed, 5 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c > b/drivers/gpu/drm/rcar-du/rcar_lvds.c > index 72a272cfc11e..99b0febc56d1 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c > @@ -712,18 +712,12 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) > { > int ret; > > - ret = drm_of_find_panel_or_bridge(lvds->dev->of_node, 1, 0, > - &lvds->panel, &lvds->next_bridge); I guess lvds->panel can be removed from the rcar_lvds struct as well? Maxime signature.asc Description: PGP signature
Re: [PATCH] drm/bridge: chipone-icn6211: switch to devm_drm_of_get_bridge
On Mon, Feb 21, 2022 at 08:42:24AM +0100, José Expósito wrote: > The function "drm_of_find_panel_or_bridge" has been deprecated in > favor of "devm_drm_of_get_bridge". > > Switch to the new function and reduce boilerplate. > > Signed-off-by: José Expósito Reviewed-by: Maxime Ripard Maxime signature.asc Description: PGP signature
Re: [PATCH V2 04/11] drm/bridge: tc358767: Implement atomic_check callback
On Sat, Feb 19, 2022 at 03:26:40AM +0100, Marek Vasut wrote: > On 2/18/22 18:34, Lucas Stach wrote: > > Hi > > [...] > > > > drivers/gpu/drm/bridge/tc358767.c | 26 ++ > > > 1 file changed, 26 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/bridge/tc358767.c > > > b/drivers/gpu/drm/bridge/tc358767.c > > > index 522c2c4d8514f..01d11adee6c74 100644 > > > --- a/drivers/gpu/drm/bridge/tc358767.c > > > +++ b/drivers/gpu/drm/bridge/tc358767.c > > > @@ -1289,6 +1289,31 @@ static bool tc_bridge_mode_fixup(struct drm_bridge > > > *bridge, > > > return true; > > > } > > > +static int tc_edp_common_atomic_check(struct drm_bridge *bridge, > > > > Drop the edp in the name here? Later in the series you call this > > function from the DPI code, so this breaks the nice clean naming > > separation from patch 1. > > > > > + struct drm_bridge_state *bridge_state, > > > + struct drm_crtc_state *crtc_state, > > > + struct drm_connector_state *conn_state, > > > + const unsigned int max_khz) > > > +{ > > > + tc_bridge_mode_fixup(bridge, &crtc_state->mode, > > > + &crtc_state->adjusted_mode); > > > + > > > + if (crtc_state->adjusted_mode.clock > max_khz) > > > + crtc_state->adjusted_mode.clock = max_khz; > > > > I don't think this is correct. The adjusted most is just for minor > > adjustments if the bridge can not fully match the mode. If userspace > > supplies a invalid high modeclock I think it would be better to fail > > the atomic check -> return -EINVAL > > Maxime was telling me that returning -EINVAL from atomic_check is weird, so > maybe we should also wait for his opinion on this part. That was in a completely different context? Our discussion was about how you would propagate clock constraints across a pipeline, and I was telling you that it would be weird to return -EINVAL for a mode that was reported on a connector as supported (or even preferred). My argument was for mode_valid to filter them out. If your clock is way above what you can support on your device, then returning an error in atomic_check is the right thing to do. Maxime signature.asc Description: PGP signature
Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver
On Sun, Feb 20, 2022 at 10:32:25PM +0100, Sam Ravnborg wrote: > Hi Noralf, > > On Sun, Feb 20, 2022 at 04:59:34PM +0100, Noralf Trønnes wrote: > > > > > > Den 19.02.2022 23.10, skrev Sam Ravnborg: > > > Hi Noralf, > > > On Fri, Feb 18, 2022 at 04:11:10PM +0100, Noralf Trønnes wrote: > > >> Add a driver that will work with most MIPI DBI compatible SPI panels. > > >> This avoids adding a driver for every new MIPI DBI compatible controller > > >> that is to be used by Linux. The 'compatible' Device Tree property with > > >> a '.bin' suffix will be used to load a firmware file that contains the > > >> controller configuration. > > > A solution where we have the command sequences as part of the DT-overlay > > > is IMO a much better way to solve this: > > > - The users needs to deal only with a single file, so there is less that > > > goes wrong > > > - The user need not reading special instructions how to handle a .bin > > > file, if the overlay is present everything is fine > > > - The file that contains the command sequences can be properly annotated > > > with comments > > > - The people that creates the command sequences has no need for a special > > > script to create the file for a display - this is all readable ascii. > > > - Using a DT-overlay the parsing of the DT-overlay can be done by > > > well-tested functions, no need to invent something new to parse the > > > file > > > > > > > > > The idea with a common mipi DBI SPI driver is super, it is the detail > > > with the .bin file that I am against. > > > > > > > The fbtft drivers has an init= DT property that contains the command > > sequence. Example for the MZ61581 display: > > > > init = <0x1b0 00 > > 0x111 > > 0x2ff > > 0x1b3 0x02 0x00 0x00 0x00 > > 0x1c0 0x13 0x3b 0x00 0x02 0x00 0x01 > > 0x00 0x43 > > 0x1c1 0x08 0x16 0x08 0x08 > > 0x1c4 0x11 0x07 0x03 0x03 > > 0x1c6 0x00 > > 0x1c8 0x03 0x03 0x13 0x5c 0x03 0x07 > > 0x14 0x08 0x00 0x21 0x08 > > 0x14 0x07 0x53 0x0c 0x13 0x03 0x03 0x21 0x00 > > 0x135 0x00 > > 0x136 0xa0 > > 0x13a 0x55 > > 0x144 0x00 0x01 > > 0x1d0 0x07 0x07 0x1d 0x03 > > 0x1d1 0x03 0x30 0x10 > > 0x1d2 0x03 0x14 0x04 > > 0x129 > > 0x12c>; > > > > Parsed here: > > https://elixir.bootlin.com/linux/latest/C/ident/fbtft_init_display_from_property > > > > Before fbdev was closed for new drivers I looked at fixing up fbtft for > > proper inclusion and asked on the Device Tree mailinglist about the init > > property and how to handle the controller configuration generically. > > I was politely told that this should be done in the driver, so when I > > made my first DRM driver I made a driver specifically for a panel > > (mi0283qt.c). > > > > Later I found another post that in clear words stated that setting > > random registers from DT was not acceptable. > > Understood and thanks for the explanation. > It is a shame that the users loose here :-( From a user point-of-view, nothing prevents the overlays and the firmware to be in the same package, provided by whatever distro or build-system they would use. The only case where it's a bit less efficient is for a panel that isn't supported already, but it's just a documentation and tooling issue, and Noralf has an awesome track record for both. And the DT syntax throw so much people off that I'm not sure it's such a benefit :) Maxime signature.asc Description: PGP signature
Re: [PATCH V2 05/11] drm/bridge: tc358767: Move hardware init to enable callback
Am Samstag, dem 19.02.2022 um 03:39 +0100 schrieb Marek Vasut: > On 2/18/22 18:49, Lucas Stach wrote: > > Am Freitag, dem 18.02.2022 um 02:00 +0100 schrieb Marek Vasut: > > > The TC358767/TC358867/TC9595 are all capable of operating either from > > > attached Xtal or from DSI clock lane clock. In case the later is used, > > > all I2C accesses will fail until the DSI clock lane is running and > > > supplying clock to the chip. > > > > > > Move all hardware initialization to enable callback to guarantee the > > > DSI clock lane is running before accessing the hardware. In case Xtal > > > is attached to the chip, this change has no effect. > > > > I'm not sure if that last statement is correct. When the chip is > > bridging to eDP, the aux channel and HPD handling is needed to be > > functional way before the atomic enable happen. I have no idea how this > > would interact with the clock supplied from the DSI lanes. Maybe it > > doesn't work at all and proper eDP support always needs a external > > reference clock? > > The driver currently assumes the TC358767 always gets RefClk from Xtal. > > There is subsequent series which adds support for deriving clock which > drive the TC358767 PLLs from DSI HS clock instead of Xtal in case the > bridge operates in DSI-to-DPI mode. That needs additional plumbing, as > the TC358767 must be able to select specific clock frequency on the DSI > HS clock lane, because its PLLs need specific frequencies, see: > > [RFC][PATCH 0/7] drm/bridge: Add support for selecting DSI host HS clock > from DSI bridge > > If someone needs to implement DSI-to-(e)DP mode without Xtal, ugh, that > would likely need to have a way to figure out the DSI HS clock frequency > already in probe and then enable those DSI HS clock very early on too ? > Probably, but that was really just something I wondered about while passing by. The real issue is that I think _this_ patch breaks eDP operation, as now the chip is initialized way too late, as the AUX channel functionality needs to be available long before the atomic bridge enable callback is called. The AUX channel is used to fetch the display EDID, so before you can even set a mode it needs to be functional. Unconditionally moving the chip init is probably (I haven't tested it yet) going to break this. Regards, Lucas > > I think we should make the "ref" clock a optional clock to properly > > describe the fact that the chip can operate without this clock in DSI > > input mode and then either do the chip init in the probe routine when > > the ref clock is present, or defer it to atomic enable when the ref > > clock is absent. > > See the RFC patchset above, that patchset does exactly that, it makes > RefClk optional. > > [...]
Re: [PATCH v4 7/9] drm: vkms: Refactor the plane composer to accept new formats
On Sun, 20 Feb 2022 22:02:12 -0300 Igor Torrente wrote: > Hi Melissa, > > On 2/9/22 18:45, Melissa Wen wrote: > > On 02/08, Igor Torrente wrote: > >> Hi Melissa, > >> > >> On 2/8/22 07:40, Melissa Wen wrote: > >>> On 01/21, Igor Torrente wrote: > Currently the blend function only accepts XRGB_ and ARGB_ > as a color input. > > This patch refactors all the functions related to the plane composition > to overcome this limitation. > > A new internal format(`struct pixel`) is introduced to deal with all > possible inputs. It consists of 16 bits fields that represent each of > the channels. > > The pixels blend is done using this internal format. And new handlers > are being added to convert a specific format to/from this internal > format. > > So the blend operation depends on these handlers to convert to this > common > format. The blended result, if necessary, is converted to the writeback > buffer format. > > This patch introduces three major differences to the blend function. > 1 - All the planes are blended at once. > 2 - The blend calculus is done as per line instead of per pixel. > 3 - It is responsible to calculates the CRC and writing the writeback > buffer(if necessary). > > These changes allow us to allocate way less memory in the intermediate > buffer to compute these operations. Because now we don't need to > have the entire intermediate image lines at once, just one line is > enough. > > | Memory consumption (output dimensions) | > |:--:| > | Current | This patch| > |:--:|:-:| > | Width * Heigth | 2 * Width | > > Beyond memory, we also have a minor performance benefit from all > these changes. Results running the IGT tests `*kms_cursor_crc*`: > > >>> First, thanks for this improvement. > >>> > >>> Some recent changes in kms_cursor_crc caused VKMS to fail in most test > >>> cases (iirc, only size-change and alpha-opaque are passing currently). > >> > >> I updated my igt and kernel(from drm_misc/drm-misc-next) to the latest > >> commit[1][2] and I'm getting mixed results. Sometimes most of the test > >> passes, sometimes almost nothing passes. > > hmm.. is it happening when running kms_cursor_crc? Is the results > > variation random or is it possible to follow a set of steps to reproduce > > it? When failing, what is the reason displayed by the log? > > I investigated it a little bit and discovered that the KMS > cursor(".*kms_cursor_crc*" ) are failing after the execution of > writeback tests(".*kms_writeback.*"). > > I don't know what is causing it, but they are failing while trying to > commit the KMS changes. > > out.txt: > IGT-Version: 1.26-NO-GIT (x86_64) (Linux: 5.17.0-rc2 x86_64) > Stack trace: >#0 ../lib/igt_core.c:1754 __igt_fail_assert() >#1 ../lib/igt_kms.c:3795 do_display_commit() >#2 ../lib/igt_kms.c:3901 igt_display_commit2() >#3 ../tests/kms_cursor_crc.c:820 __igt_uniquereal_main814() >#4 ../tests/kms_cursor_crc.c:814 main() >#5 ../csu/libc-start.c:308 __libc_start_main() >#6 [_start+0x2a] > Subtest pipe-A-cursor-size-change: FAIL > > err.txt: > (kms_cursor_crc:1936) igt_kms-CRITICAL: Test assertion failure function > do_display_commit, file ../lib/igt_kms.c:3795: > (kms_cursor_crc:1936) igt_kms-CRITICAL: Failed assertion: ret == 0 > (kms_cursor_crc:1936) igt_kms-CRITICAL: Last errno: 22, Invalid argument > (kms_cursor_crc:1936) igt_kms-CRITICAL: error: -22 != 0 > > > > > From my side, only the first two subtest of kms_cursor_crc is passing > > before this patch. And after your changes here, all subtests are > > successful again, except those related to 32x10 cursor size (that needs > > futher investigation). I didn't check how the recent changes in > > kms_cursor_crc affect VKMS performance on it, but I bet that clearing > > the alpha channel is the reason to have the performance back. > > Yeah, I also don't understand why the 32x10 cursor tests are failing. > Hi, are the tests putting the cursor partially outside of the CRTC area? Or partially outside of primary plane area (which IIRC you used when you should have used the CRTC area?)? Does the writeback test forget to unlink the writeback connector? Or does VKMS not handle unlinking the writeback connector? If both are a problem, the latter would be just an unrelated bug that exposes the first bug in VKMS, because whether writeback is used or not probably should not affect where the cursor plane is allowed to be. Thanks, pq pgpsMKjczohKx.pgp Description: OpenPGP digital signature
Re: [PATCH v10 1/4] MIPS: Loongson64: dts: update the display controller device node
On 2/20/22 5:55 PM, Sui Jingfeng wrote: > From: suijingfeng > > The display controller is a pci device, its PCI vendor id is 0x0014 > its PCI device id is 0x7a06. > > 1) In order to let the driver to know which chip the DC is contained >in, the compatible string of the display controller is updated >according to the chip's name. > > 2) Add display controller device node for ls2k1000 SoC > > Reported-by: Krzysztof Kozlowski > Signed-off-by: suijingfeng > Signed-off-by: Sui Jingfeng <15330273...@189.cn> > --- > arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi | 8 > arch/mips/boot/dts/loongson/ls7a-pch.dtsi | 7 ++- > 2 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi > b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi > index 768cf2abcea3..af9cda540f9e 100644 > --- a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi > +++ b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi > @@ -209,6 +209,14 @@ gpu@5,0 { > interrupt-parent = <&liointc0>; > }; > > + lsdc: display-controller@6,0 { Shouldn't the node name just be "display", according to the section 2.2.2 of the DT spec? [...] > diff --git a/arch/mips/boot/dts/loongson/ls7a-pch.dtsi > b/arch/mips/boot/dts/loongson/ls7a-pch.dtsi > index 2f45fce2cdc4..ec35ea9b2fe8 100644 > --- a/arch/mips/boot/dts/loongson/ls7a-pch.dtsi > +++ b/arch/mips/boot/dts/loongson/ls7a-pch.dtsi > @@ -160,11 +160,8 @@ gpu@6,0 { > interrupt-parent = <&pic>; > }; > > - dc@6,1 { > - compatible = "pci0014,7a06.0", > -"pci0014,7a06", > -"pciclass03", > -"pciclass0300"; > + lsdc: display-controller@6,1 { Same here... [...] MBR, Sergey
Re: [PULL] drm-intel-gt-next
On Mon, 21 Feb 2022, Dave Airlie wrote: > On Thu, 17 Feb 2022 at 20:26, Joonas Lahtinen > wrote: >> >> Hi Dave & Daniel, >> >> Here is the first drm-intel-gt-next feature PR towards v5.18. > > Am I missing some previous drm-intel pull? > > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/i915/gt/intel_workarounds.c: > In function ‘rcs_engine_wa_init’: > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/i915/gt/intel_workarounds.c:2051:40: > error: ‘XEHP_DIS_BBL_SYSPIPE’ undeclared (first use in this function) > 2051 | wa_masked_en(wal, GEN9_ROW_CHICKEN4, XEHP_DIS_BBL_SYSPIPE); > |^~~~ > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/i915/gt/intel_workarounds.c:2051:40: > note: each undeclared identifier is reported only once for each > function it appears in There's apparently a silent conflict between changes in drm-intel-next and drm-intel-gt-next. There's a fixup patch in drm-rerere: fixups/drm-intel-gt-next.patch. We opted to sync the branches via drm-next pulls and backmerges, but I'm afraid that means you'd have to use the fixups when merging. I guess we failed to communicate that. The alternative would've been cross-merges within drm-intel. BR, Jani. > > Dave. >> >> For DG2 adds subplatform G12, missing workarounds and fixes GuC >> loading on ARM64. C0/D0 stepping info added for RPL-S. >> >> For uAPI enables support for simple parallel submission with >> execlists which was previously enabled only for GuC. >> >> Further fixes for PMU metrics when GuC is enabled, better error >> reporting for GuC loading failures. Fix for PXP unbind splat. >> Updates to GuC version 69.0.3 with improvements to GT reset >> scenarios. >> >> The rest is mostly refactoring of the memory management code, >> as highlights introduction of async unbinding/migration and >> removal of short-term pinning in execbuf. >> >> Then a few selftest and coding style fixes. >> >> Regards, Joonas >> >> *** >> >> drm-intel-gt-next-2022-02-17: >> >> UAPI Changes: >> >> - Weak parallel submission support for execlists >> >> Minimal implementation of the parallel submission support for >> execlists backend that was previously only implemented for GuC. >> Support one sibling non-virtual engine. >> >> Core Changes: >> >> - Two backmerges of drm/drm-next for header file renames/changes and >> i915_regs reorganization >> >> Driver Changes: >> >> - Add new DG2 subplatform: DG2-G12 (Matt R) >> - Add new DG2 workarounds (Matt R, Ram, Bruce) >> - Handle pre-programmed WOPCM registers for DG2+ (Daniele) >> - Update guc shim control programming on XeHP SDV+ (Daniele) >> - Add RPL-S C0/D0 stepping information (Anusha) >> - Improve GuC ADS initialization to work on ARM64 on dGFX (Lucas) >> >> - Fix KMD and GuC race on accessing PMU busyness (Umesh) >> - Use PM timestamp instead of RING TIMESTAMP for reference in PMU with GuC >> (Umesh) >> - Report error on invalid reset notification from GuC (John) >> - Avoid WARN splat by holding RPM wakelock during PXP unbind (Juston) >> - Fixes to parallel submission implementation (Matt B.) >> - Improve GuC loading status check/error reports (John) >> - Tweak TTM LRU priority hint selection (Matt A.) >> - Align the plane_vma to min_page_size of stolen mem (Ram) >> >> - Introduce vma resources and implement async unbinding (Thomas) >> - Use struct vma_resource instead of struct vma_snapshot (Thomas) >> - Return some TTM accel move errors instead of trying memcpy move (Thomas) >> - Fix a race between vma / object destruction and unbinding (Thomas) >> - Remove short-term pins from execbuf (Maarten) >> - Update to GuC version 69.0.3 (John, Michal Wa.) >> - Improvements to GT reset paths in GuC backend (Matt B.) >> - Use shrinker_release_pages instead of writeback in shmem object hooks >> (Matt A., Tvrtko) >> - Use trylock instead of blocking lock when freeing GEM objects (Maarten) >> - Allocate intel_engine_coredump_alloc with ALLOW_FAIL (Matt B.) >> - Fixes to object unmapping and purging (Matt A) >> - Check for wedged device in GuC backend (John) >> - Avoid lockdep splat by locking dpt_obj around set_cache_level (Maarten) >> - Allow dead vm to unbind vma's without lock (Maarten) >> - s/engine->i915/i915/ for DG2 engine workarounds (Matt R) >> >> - Use to_gt() helper for GGTT accesses (Michal Wi.) >> - Selftest improvements (Matt B., Thomas, Ram) >> - Coding style and compiler warning fixes (Matt B., Jasmine, Andi, Colin, >> Gustavo, Dan) >> >> The following changes since commit 53dbee4926d3706ca9e03f3928fa85b5ec3bc0cc: >> >> Merge tag 'drm-misc-next-2022-01-27' of >> git://anongit.freedesktop.org/drm/drm-misc into drm-next (2022-02-01 >> 19:02:41 +1000) >> >> are available in the Git repository at: >> >> git://anongit.freedesktop.org/drm/drm-intel >> tags/drm-intel-gt-next-2022-02-17 >> >> for you to fetch changes up to 154cfae6158141b18d65abb0db679bb51a8294e7: >> >> drm/i915/dg2: Add Wa_22011100796 (2022-02-11 17:11:44
Re: [PATCH v8 13/19] drm/mediatek: dpi: Add dpintf support
Hi (Now I remember your series ;) On Fri, Feb 18, 2022 at 03:54:31PM +0100, Guillaume Ranquet wrote: > dpintf is the displayport interface hardware unit. This unit is similar > to dpi and can reuse most of the code. > > This patch adds support for mt8195-dpintf to this dpi driver. Main > differences are: > - Some features/functional components are not available for dpintf >which are now excluded from code execution once is_dpintf is set > - dpintf can and needs to choose between different clockdividers based >on the clockspeed. This is done by choosing a different clock parent. > - There are two additional clocks that need to be managed. These are >only set for dpintf and will be set to NULL if not supplied. The >clk_* calls handle these as normal clocks then. > - Some register contents differ slightly between the two components. To >work around this I added register bits/masks with a DPINTF_ prefix >and use them where different. > > Based on a separate driver for dpintf created by > Jason-JH.Lin . > > Signed-off-by: Markus Schneider-Pargmann > Signed-off-by: Guillaume Ranquet > --- > drivers/gpu/drm/mediatek/mtk_dpi.c | 164 +--- > drivers/gpu/drm/mediatek/mtk_dpi_regs.h | 38 + > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 8 + > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 + > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 +- > include/linux/soc/mediatek/mtk-mmsys.h | 2 + > 6 files changed, 198 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c > b/drivers/gpu/drm/mediatek/mtk_dpi.c > index be99399faf1bb..c5639ada868f8 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c > @@ -63,6 +63,14 @@ enum mtk_dpi_out_color_format { > MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL > }; > > +enum TVDPLL_CLK { > + TVDPLL_PLL = 0, > + TVDPLL_D2 = 2, > + TVDPLL_D4 = 4, > + TVDPLL_D8 = 8, > + TVDPLL_D16 = 16, > +}; > + > struct mtk_dpi { > struct drm_encoder encoder; > struct drm_bridge bridge; > @@ -71,8 +79,10 @@ struct mtk_dpi { > void __iomem *regs; > struct device *dev; > struct clk *engine_clk; > + struct clk *dpi_ck_cg; > struct clk *pixel_clk; > struct clk *tvd_clk; > + struct clk *pclk_src[5]; > int irq; > struct drm_display_mode mode; > const struct mtk_dpi_conf *conf; > @@ -126,6 +136,7 @@ struct mtk_dpi_conf { > const u32 *output_fmts; > u32 num_output_fmts; > bool is_ck_de_pol; > + bool is_dpintf; > bool swap_input_support; > // Mask used for HWIDTH, HPORCH, VSYNC_WIDTH and VSYNC_PORCH (no shift) > u32 dimension_mask; > @@ -384,6 +395,25 @@ static void mtk_dpi_config_disable_edge(struct mtk_dpi > *dpi) > mtk_dpi_mask(dpi, dpi->conf->reg_h_fre_con, 0, EDGE_SEL_EN); > } > > +static void mtk_dpi_matrix_sel(struct mtk_dpi *dpi, enum > mtk_dpi_out_color_format format) > +{ > + u32 matrix_sel = 0; > + > + switch (format) { > + case MTK_DPI_COLOR_FORMAT_YCBCR_422: > + case MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL: > + case MTK_DPI_COLOR_FORMAT_YCBCR_444: > + case MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL: > + case MTK_DPI_COLOR_FORMAT_XV_YCC: > + if (dpi->mode.hdisplay <= 720) > + matrix_sel = 0x2; > + break; > + default: > + break; > + } > + mtk_dpi_mask(dpi, DPI_MATRIX_SET, matrix_sel, INT_MATRIX_SEL_MASK); > +} > + > static void mtk_dpi_config_color_format(struct mtk_dpi *dpi, > enum mtk_dpi_out_color_format format) > { > @@ -391,6 +421,7 @@ static void mtk_dpi_config_color_format(struct mtk_dpi > *dpi, > (format == MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL)) { > mtk_dpi_config_yuv422_enable(dpi, false); > mtk_dpi_config_csc_enable(dpi, true); > + mtk_dpi_matrix_sel(dpi, format); > if (dpi->conf->swap_input_support) > mtk_dpi_config_swap_input(dpi, false); > mtk_dpi_config_channel_swap(dpi, MTK_DPI_OUT_CHANNEL_SWAP_BGR); > @@ -398,6 +429,7 @@ static void mtk_dpi_config_color_format(struct mtk_dpi > *dpi, > (format == MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL)) { > mtk_dpi_config_yuv422_enable(dpi, true); > mtk_dpi_config_csc_enable(dpi, true); > + mtk_dpi_matrix_sel(dpi, format); > if (dpi->conf->swap_input_support) > mtk_dpi_config_swap_input(dpi, true); > mtk_dpi_config_channel_swap(dpi, MTK_DPI_OUT_CHANNEL_SWAP_RGB); > @@ -438,6 +470,8 @@ static void mtk_dpi_power_off(struct mtk_dpi *dpi) > mtk_dpi_disable(dpi); > clk_disable_unprepare(dpi->pixel_clk); > clk_disable_unprepare(dpi->engine_clk); > + clk_disable_unprepare(dpi->dpi_ck_cg); > + clk_disable_unprepa
Re: [PATCH v3] simplefb: Enable boot time VESA graphic mode selection.
Hi Am 18.02.22 um 17:04 schrieb Michal Suchanek: Since switch to simplefb/simpledrm VESA graphic modes are no longer available with legacy BIOS. The x86 realmode boot code enables the VESA graphic modes when option FB_BOOT_VESA_SUPPORT is enabled. To enable use of VESA modes with simplefb in legacy BIOS boot mode drop dependency of BOOT_VESA_SUPPORT on FB, also drop the FB_ prefix, and select the option when simplefb enabled on x86. The BOOT_VESA_SUPPORT is not specific to framebuffer but rather to x86 platform, move it from fbdev to x86 Kconfig. Fixes: e3263ab389a7 ("x86: provide platform-devices for boot-framebuffers") Signed-off-by: Michal Suchanek Acked-by: Thomas Zimmermann Thanks for the patch. I'll wait a bit for additional reviews before merging it. Best regards Thomas --- v2: Select BOOT_VESA_SUPPORT from simplefb rather than simpledrm. The simpledrm driver uses the firmware provided video modes only indirectly through simplefb, and both can be enabled independently. v3: Move BOOT_VESA_SUPPORT from fbdev to x86 --- arch/x86/Kconfig| 6 ++ arch/x86/boot/video-vesa.c | 4 ++-- drivers/firmware/Kconfig| 1 + drivers/video/fbdev/Kconfig | 13 +++-- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 9f5bd41bf660..cceb1aab0486 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -942,6 +942,12 @@ config GART_IOMMU If unsure, say Y. +config BOOT_VESA_SUPPORT + bool + help + If true, at least one selected framebuffer driver can take advantage + of VESA video modes set at an early boot stage via the vga= parameter. + config MAXSMP bool "Enable Maximum number of SMP Processors and NUMA Nodes" depends on X86_64 && SMP && DEBUG_KERNEL diff --git a/arch/x86/boot/video-vesa.c b/arch/x86/boot/video-vesa.c index 7e185977a984..c2c6d35e3a43 100644 --- a/arch/x86/boot/video-vesa.c +++ b/arch/x86/boot/video-vesa.c @@ -83,7 +83,7 @@ static int vesa_probe(void) (vminfo.memory_layout == 4 || vminfo.memory_layout == 6) && vminfo.memory_planes == 1) { -#ifdef CONFIG_FB_BOOT_VESA_SUPPORT +#ifdef CONFIG_BOOT_VESA_SUPPORT /* Graphics mode, color, linear frame buffer supported. Only register the mode if if framebuffer is configured, however, @@ -121,7 +121,7 @@ static int vesa_set_mode(struct mode_info *mode) if ((vminfo.mode_attr & 0x15) == 0x05) { /* It's a supported text mode */ is_graphic = 0; -#ifdef CONFIG_FB_BOOT_VESA_SUPPORT +#ifdef CONFIG_BOOT_VESA_SUPPORT } else if ((vminfo.mode_attr & 0x99) == 0x99) { /* It's a graphics mode with linear frame buffer */ is_graphic = 1; diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index 75cb91055c17..ad64f3a6f54f 100644 --- a/drivers/firmware/Kconfig +++ b/drivers/firmware/Kconfig @@ -224,6 +224,7 @@ config SYSFB config SYSFB_SIMPLEFB bool "Mark VGA/VBE/EFI FB as generic system framebuffer" depends on SYSFB + select BOOT_VESA_SUPPORT help Firmwares often provide initial graphics framebuffers so the BIOS, bootloader or kernel can show basic video-output during boot for diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig index 6ed5e608dd04..5bdd303b5268 100644 --- a/drivers/video/fbdev/Kconfig +++ b/drivers/video/fbdev/Kconfig @@ -66,13 +66,6 @@ config FB_DDC select I2C_ALGOBIT select I2C -config FB_BOOT_VESA_SUPPORT - bool - depends on FB - help - If true, at least one selected framebuffer driver can take advantage - of VESA video modes set at an early boot stage via the vga= parameter. - config FB_CFB_FILLRECT tristate depends on FB @@ -627,7 +620,7 @@ config FB_VESA select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT - select FB_BOOT_VESA_SUPPORT + select BOOT_VESA_SUPPORT help This is the frame buffer device driver for generic VESA 2.0 compliant graphic cards. The older VESA 1.2 cards are not supported. @@ -1051,7 +1044,7 @@ config FB_INTEL select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT - select FB_BOOT_VESA_SUPPORT if FB_INTEL = y + select BOOT_VESA_SUPPORT if FB_INTEL = y depends on !DRM_I915 help This driver supports the on-board graphics built in to the Intel @@ -1378,7 +1371,7 @@ config FB_SIS select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT - select FB_BOOT_VESA_SUPPORT if FB_SIS = y + select BOOT_VESA_SUPPORT if FB_SIS = y select FB_SIS_300 if !FB_SIS_315 help This
Re: [PATCH v6 17/23] arm64: dts: rockchip: rk356x: Add HDMI nodes
On Mon, Feb 21, 2022 at 09:54:28AM +0100, Frank Wunderlich wrote: > Hi > > > Gesendet: Donnerstag, 17. Februar 2022 um 09:29 Uhr > > Von: "Sascha Hauer" > > > --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi > > +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi > > @@ -10,7 +10,6 @@ > > #include > > #include > > #include > > -#include > > why dropping this after adding in part 16? > > > #include > > it looks like you moved this to board includes...imho this should stay > in the rk356x.dtsi, because compilation will fail if a board without > the vop2 (and missing the include) is derived from rk356x.dtsi. I dropped adding the include from Patch 16. The include is not needed by rk356x.dtsi. When a board without vop2 support is added then it won't need the include either. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[PATCH v1 7/8] ARM: dts: imx6dl-victgo: The TGO uses a lg, lb070wv8 compatible 7" display
From: Robin van der Gracht This series of devices is using lg,lb070wv8 instead of kyo,tcg121xglp. Signed-off-by: Robin van der Gracht Signed-off-by: Oleksij Rempel --- arch/arm/boot/dts/imx6dl-victgo.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/imx6dl-victgo.dts b/arch/arm/boot/dts/imx6dl-victgo.dts index 907682248aa7..7839021bc3eb 100644 --- a/arch/arm/boot/dts/imx6dl-victgo.dts +++ b/arch/arm/boot/dts/imx6dl-victgo.dts @@ -129,7 +129,7 @@ led-2 { }; panel { - compatible = "kyo,tcg121xglp"; + compatible = "lg,lb070wv8"; backlight = <&backlight_lcd>; power-supply = <®_3v3>; -- 2.30.2
[PATCH v1 6/8] ARM: dts: imx6dl-victgo: Add interrupt-counter nodes
From: Robin van der Gracht Interrupt counter is mainlined, now we can add missing counter nodes. Signed-off-by: Robin van der Gracht Signed-off-by: Oleksij Rempel --- arch/arm/boot/dts/imx6dl-victgo.dts | 41 - 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/imx6dl-victgo.dts b/arch/arm/boot/dts/imx6dl-victgo.dts index 20c7f80e5ec9..907682248aa7 100644 --- a/arch/arm/boot/dts/imx6dl-victgo.dts +++ b/arch/arm/boot/dts/imx6dl-victgo.dts @@ -54,6 +54,27 @@ comp0_out: endpoint { }; }; + counter-0 { + compatible = "interrupt-counter"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_counter0>; + gpios = <&gpio2 0 GPIO_ACTIVE_LOW>; + }; + + counter-1 { + compatible = "interrupt-counter"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_counter1>; + gpios = <&gpio2 1 GPIO_ACTIVE_LOW>; + }; + + counter-2 { + compatible = "interrupt-counter"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_counter2>; + gpios = <&gpio2 2 GPIO_ACTIVE_LOW>; + }; + gpio-keys { compatible = "gpio-keys"; pinctrl-names = "default"; @@ -400,7 +421,7 @@ &gpio1 { &gpio2 { gpio-line-names = - "", "", "", "", "", "", "", "", + "YACO_WHEEL", "YACO_RADAR", "YACO_PTO", "", "", "", "", "", "", "LED_PWM", "", "", "", "", "", "", "", "", "", "", "", "", "ISB_IN1", "ON_SWITCH", @@ -706,6 +727,24 @@ MX6QDL_PAD_KEY_ROW3__GPIO4_IO130x13008 >; }; + pinctrl_counter0: counter0grp { + fsl,pins = < + MX6QDL_PAD_NANDF_D0__GPIO2_IO00 0x1b000 + >; + }; + + pinctrl_counter1: counter1grp { + fsl,pins = < + MX6QDL_PAD_NANDF_D1__GPIO2_IO01 0x1b000 + >; + }; + + pinctrl_counter2: counter2grp { + fsl,pins = < + MX6QDL_PAD_NANDF_D2__GPIO2_IO02 0x1b000 + >; + }; + pinctrl_ecspi1: ecspi1grp { fsl,pins = < MX6QDL_PAD_EIM_D17__ECSPI1_MISO 0x100b1 -- 2.30.2
[PATCH v1 1/8] ARM: dts: imx6qdl-vicut1/vicutgo: Set default backlight brightness to maximum
From: David Jander Recover default behavior of the device and set maximal brightness Signed-off-by: David Jander Signed-off-by: Oleksij Rempel --- arch/arm/boot/dts/imx6dl-victgo.dts | 2 +- arch/arm/boot/dts/imx6qdl-vicut1.dtsi | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/imx6dl-victgo.dts b/arch/arm/boot/dts/imx6dl-victgo.dts index 227c952543d4..e6134efbfabd 100644 --- a/arch/arm/boot/dts/imx6dl-victgo.dts +++ b/arch/arm/boot/dts/imx6dl-victgo.dts @@ -28,7 +28,7 @@ backlight: backlight { pwms = <&pwm1 0 500 0>; brightness-levels = <0 16 64 255>; num-interpolated-steps = <16>; - default-brightness-level = <1>; + default-brightness-level = <48>; power-supply = <®_3v3>; enable-gpios = <&gpio4 28 GPIO_ACTIVE_HIGH>; }; diff --git a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi index 1ac7e13249d2..c1d06bc28c67 100644 --- a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi +++ b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi @@ -23,7 +23,7 @@ backlight: backlight { pwms = <&pwm1 0 500 0>; brightness-levels = <0 16 64 255>; num-interpolated-steps = <16>; - default-brightness-level = <1>; + default-brightness-level = <48>; power-supply = <®_3v3>; enable-gpios = <&gpio4 28 GPIO_ACTIVE_HIGH>; }; -- 2.30.2
[PATCH v1 5/8] ARM: dts: imx6qdl-vicut1/vicutgo: The sgtl5000 uses i2s not ac97
From: Robin van der Gracht According to Documentation/devicetree/bindings/sound/fsl,ssi.txt 'fsl,mode' should be specified for AC97 mode only. The 'fsl,ssi' documentation doesn't say anything about specifying 'sound-dai-cells' so we'll remove that as well. Signed-off-by: Robin van der Gracht Signed-off-by: Oleksij Rempel --- arch/arm/boot/dts/imx6dl-victgo.dts | 2 -- arch/arm/boot/dts/imx6qdl-vicut1.dtsi | 2 -- 2 files changed, 4 deletions(-) diff --git a/arch/arm/boot/dts/imx6dl-victgo.dts b/arch/arm/boot/dts/imx6dl-victgo.dts index d542ddad4e32..20c7f80e5ec9 100644 --- a/arch/arm/boot/dts/imx6dl-victgo.dts +++ b/arch/arm/boot/dts/imx6dl-victgo.dts @@ -591,8 +591,6 @@ &pwm3 { }; &ssi1 { - #sound-dai-cells = <0>; - fsl,mode = "ac97-slave"; status = "okay"; }; diff --git a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi index ec39008c0950..97ef8264947a 100644 --- a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi +++ b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi @@ -466,8 +466,6 @@ &pwm3 { }; &ssi1 { - #sound-dai-cells = <0>; - fsl,mode = "ac97-slave"; status = "okay"; }; -- 2.30.2
[PATCH v1 4/8] ARM: dts: imx6qdl-vicut1: update gpio-line-names for some GPIOs
From: David Jander countedX lines have different board names (YACO_x). And REV_ and BOARD_ pins have multiple functions. So, use names exposed to the OS. Signed-off-by: David Jander Signed-off-by: Oleksij Rempel --- arch/arm/boot/dts/imx6qdl-vicut1.dtsi | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi index 2f6b263eea66..ec39008c0950 100644 --- a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi +++ b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi @@ -277,9 +277,9 @@ &gpio1 { &gpio2 { gpio-line-names = - "count0", "count1", "count2", "", "", "", "", "", - "REV_ID0", "REV_ID1", "REV_ID2", "REV_ID3", "REV_ID4", - "BOARD_ID0", "BOARD_ID1", "BOARD_ID2", + "YACO_WHEEL", "YACO_RADAR", "YACO_PTO", "", "", "", "", "", + "", "LED_PWM", "", "", "", + "", "", "", "", "", "", "", "", "", "", "ON_SWITCH", "POWER_LED", "", "ECSPI2_SS0", "", "", "", "", ""; }; @@ -298,8 +298,10 @@ &gpio4 { "", "", "", "", "", "", "UART4_TXD", "UART4_RXD", "UART5_TXD", "UART5_RXD", "CAN1_TX", "CAN1_RX", "CAN1_SR", "CAN2_SR", "CAN2_TX", "CAN2_RX", - "LED_DI0_DEBUG_0", "LED_DI0_DEBUG_1", "", "", "", "", "", "", - "", "", "", "", "BL_EN", "BL_PWM", "", ""; + "LED_DI0_DEBUG_0", "LED_DI0_DEBUG_1", "", "", "", "ON1_CTRL", + "ON2_CTRL", "HITCH_IN_OUT", + "LIGHT_ON", "", "", "CONTACT_IN", "BL_EN", "BL_PWM", "", + "ISB_LED"; }; &gpio5 { -- 2.30.2
[PATCH v1 3/8] ARM: dts: imx6qdl-vicut1/vicutgo: Add backlight_led node
From: David Jander backlight_led is the dimmable backlight for the rubber border on the case. It is also used to highlight the power- and some other buttons. MX6QDL_PAD_SD4_DAT1__PWM3_OUT is also assigned as output for pwm3. Since we need pwm3 for the backlight, we're forced to disable user space hardware revision detection. The bootloader will have to supply this information (i.e. through device tree). Signed-off-by: David Jander Signed-off-by: Oleksij Rempel --- arch/arm/boot/dts/imx6dl-victgo.dts | 24 +-- arch/arm/boot/dts/imx6qdl-vicut1.dtsi | 33 +-- 2 files changed, 32 insertions(+), 25 deletions(-) diff --git a/arch/arm/boot/dts/imx6dl-victgo.dts b/arch/arm/boot/dts/imx6dl-victgo.dts index 833340c30537..d542ddad4e32 100644 --- a/arch/arm/boot/dts/imx6dl-victgo.dts +++ b/arch/arm/boot/dts/imx6dl-victgo.dts @@ -33,6 +33,15 @@ backlight_lcd: backlight { enable-gpios = <&gpio4 28 GPIO_ACTIVE_HIGH>; }; + backlight_led: backlight_led { + compatible = "pwm-backlight"; + pwms = <&pwm3 0 500 0>; + brightness-levels = <0 16 64 255>; + num-interpolated-steps = <16>; + default-brightness-level = <48>; + power-supply = <®_3v3>; + }; + connector { compatible = "composite-video-connector"; label = "Composite0"; @@ -392,8 +401,8 @@ &gpio1 { &gpio2 { gpio-line-names = "", "", "", "", "", "", "", "", - "REV_ID0", "REV_ID1", "REV_ID2", "REV_ID3", "REV_ID4", - "BOARD_ID0", "BOARD_ID1", "BOARD_ID2", + "", "LED_PWM", "", "", "", + "", "", "", "", "", "", "", "", "", "ISB_IN1", "ON_SWITCH", "POWER_LED", "", "", "", "", "", "", ""; }; @@ -778,17 +787,6 @@ MX6QDL_PAD_CSI0_VSYNC__GPIO5_IO21 0x1f0b0 /* ITU656_nPDN */ MX6QDL_PAD_CSI0_DATA_EN__GPIO5_IO20 0x1b0b0 - /* HW revision detect */ - /* REV_ID0 */ - MX6QDL_PAD_SD4_DAT0__GPIO2_IO08 0x1b0b0 - /* REV_ID1 is shared with PWM3 */ - /* REV_ID2 */ - MX6QDL_PAD_SD4_DAT2__GPIO2_IO10 0x1b0b0 - /* REV_ID3 */ - MX6QDL_PAD_SD4_DAT3__GPIO2_IO11 0x1b0b0 - /* REV_ID4 */ - MX6QDL_PAD_SD4_DAT4__GPIO2_IO12 0x1b0b0 - /* New in HW revision 1 */ /* ON1_FB */ MX6QDL_PAD_EIM_D20__GPIO3_IO20 0x100b0 diff --git a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi index a1fbbc9c26b6..2f6b263eea66 100644 --- a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi +++ b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi @@ -28,6 +28,15 @@ backlight_lcd: backlight { enable-gpios = <&gpio4 28 GPIO_ACTIVE_HIGH>; }; + backlight_led: backlight_led { + compatible = "pwm-backlight"; + pwms = <&pwm3 0 500 0>; + brightness-levels = <0 16 64 255>; + num-interpolated-steps = <16>; + default-brightness-level = <48>; + power-supply = <®_3v3>; + }; + connector { compatible = "composite-video-connector"; label = "Composite0"; @@ -448,6 +457,12 @@ &pwm1 { status = "okay"; }; +&pwm3 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_pwm3>; + status = "okay"; +}; + &ssi1 { #sound-dai-cells = <0>; fsl,mode = "ac97-slave"; @@ -669,18 +684,6 @@ MX6QDL_PAD_CSI0_VSYNC__GPIO5_IO21 0x1f0b0 /* ITU656_nPDN */ MX6QDL_PAD_CSI0_DATA_EN__GPIO5_IO20 0x1b0b0 - /* HW revision detect */ - /* REV_ID0 */ - MX6QDL_PAD_SD4_DAT0__GPIO2_IO08 0x1b0b0 - /* REV_ID1 */ - MX6QDL_PAD_SD4_DAT1__GPIO2_IO09 0x1b0b0 - /* REV_ID2 */ - MX6QDL_PAD_SD4_DAT2__GPIO2_IO10 0x1b0b0 - /* REV_ID3 */ - MX6QDL_PAD_SD4_DAT3__GPIO2_IO11 0x1b0b0 - /* REV_ID4 */ - MX6QDL_PAD_SD4_DAT4__GPIO2_IO12 0x1b0b0 - /* New in HW revision 1 */ /* ON1_FB */ MX6QDL_PAD_EIM_D20__GPIO3_IO20 0x100b0 @@ -738,6 +741,12 @@ MX6QDL_PAD_DISP0_DAT8__PWM1_OUT0x1b0b0 >; }; + pinctr
[PATCH v1 8/8] ARM: dts: imx6qdl-victgo: add CAN termination support
From: David Jander The gpio1 0 pin is controlling CAN termination, not USB H1 VBUS. So, remove wrong regulator and assign this gpio to new DT CAN termination property. Signed-off-by: David Jander Signed-off-by: Oleksij Rempel --- arch/arm/boot/dts/imx6dl-victgo.dts | 12 ++-- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/arch/arm/boot/dts/imx6dl-victgo.dts b/arch/arm/boot/dts/imx6dl-victgo.dts index 7839021bc3eb..d66da630e0af 100644 --- a/arch/arm/boot/dts/imx6dl-victgo.dts +++ b/arch/arm/boot/dts/imx6dl-victgo.dts @@ -160,15 +160,6 @@ reg_3v3: regulator-3v3 { regulator-max-microvolt = <330>; }; - reg_h1_vbus: regulator-h1-vbus { - compatible = "regulator-fixed"; - regulator-name = "h1-vbus"; - regulator-min-microvolt = <500>; - regulator-max-microvolt = <500>; - gpio = <&gpio1 0 GPIO_ACTIVE_HIGH>; - enable-active-high; - }; - reg_otg_vbus: regulator-otg-vbus { compatible = "regulator-fixed"; regulator-name = "otg-vbus"; @@ -312,6 +303,8 @@ IMX_AUDMUX_V2_PTCR_SYN IMX_AUDMUX_V2_PDCR_RXDSEL(0) &can1 { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_can1>; + termination-gpios = <&gpio1 0 GPIO_ACTIVE_LOW>; + termination-ohms = <150>; status = "okay"; }; @@ -646,7 +639,6 @@ &uart5 { }; &usbh1 { - vbus-supply = <®_h1_vbus>; pinctrl-names = "default"; phy_type = "utmi"; dr_mode = "host"; -- 2.30.2
[PATCH v1 2/8] ARM: dts: imx6qdl-vicut1/vicutgo: Rename backlight to backlight_lcd
From: David Jander We have two backlight sources on this boards. Use more specific name for the LCD backlight to see the difference. Signed-off-by: David Jander Signed-off-by: Robin van der Gracht Signed-off-by: Oleksij Rempel --- arch/arm/boot/dts/imx6dl-victgo.dts | 4 ++-- arch/arm/boot/dts/imx6qdl-vicut1.dtsi | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/arm/boot/dts/imx6dl-victgo.dts b/arch/arm/boot/dts/imx6dl-victgo.dts index e6134efbfabd..833340c30537 100644 --- a/arch/arm/boot/dts/imx6dl-victgo.dts +++ b/arch/arm/boot/dts/imx6dl-victgo.dts @@ -21,7 +21,7 @@ chosen { stdout-path = &uart4; }; - backlight: backlight { + backlight_lcd: backlight { compatible = "pwm-backlight"; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_backlight>; @@ -100,7 +100,7 @@ led-2 { panel { compatible = "kyo,tcg121xglp"; - backlight = <&backlight>; + backlight = <&backlight_lcd>; power-supply = <®_3v3>; port { diff --git a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi index c1d06bc28c67..a1fbbc9c26b6 100644 --- a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi +++ b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi @@ -16,7 +16,7 @@ chosen { stdout-path = &uart4; }; - backlight: backlight { + backlight_lcd: backlight { compatible = "pwm-backlight"; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_backlight>; @@ -102,7 +102,7 @@ led-2 { panel { compatible = "kyo,tcg121xglp"; - backlight = <&backlight>; + backlight = <&backlight_lcd>; power-supply = <®_3v3>; port { -- 2.30.2
Re: [PATCH v16 21/40] pwm: tegra: Add runtime PM and OPP support
Hello Uwe, 21.02.2022 11:17, Uwe Kleine-König пишет: >> @@ -344,7 +387,10 @@ static const struct of_device_id tegra_pwm_of_match[] = >> { >> MODULE_DEVICE_TABLE(of, tegra_pwm_of_match); >> >> static const struct dev_pm_ops tegra_pwm_pm_ops = { >> -SET_SYSTEM_SLEEP_PM_OPS(tegra_pwm_suspend, tegra_pwm_resume) >> +SET_RUNTIME_PM_OPS(tegra_pwm_runtime_suspend, tegra_pwm_runtime_resume, >> + NULL) >> +SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, >> +pm_runtime_force_resume) >> }; >> >> static struct platform_driver tegra_pwm_driver = { > I admit to not completely understand the effects of this patch, but I > don't see a problem either. So for me this patch is OK: > > Acked-by: Uwe Kleine-König > > I spot a problem, it's not introduced by this patch however: If the > consumer of the PWM didn't stop the hardware, the suspend should IMHO be > prevented. Why? The PWM driver itself will stop the h/w on suspend. > I wonder if the patches in this series go in in one go via an ARM or > Tegra tree, or each patch via its respective maintainer tree. This series, including this patch, was already applied to 5.17 via the tegra/soc tree. No action is needed anymore.
Re: [PATCH v4 0/10] clk: Improve clock range handling
Hi Stephen, (CC'ing Jean-Michel) On Fri, Feb 18, 2022 at 06:24:08PM -0800, Stephen Boyd wrote: > Quoting Laurent Pinchart (2022-02-14 01:45:56) > > Hi Maxime and Stephen, > > > > We have recently posted a driver for the BCM2711 Unicam CSI-2 receiver > > (see [1]) which is a perfect candidate for this API, as it needs a > > minimum rate for the VPU clock. Any chance we can get this series merged > > ? :-) > > The rate range API already exists, but it's busted. I can see you like > the patch series, can you provide any Reviewed-by or Tested-by tags? Jean-Michel, as you're working on upstreaming the RPi Unicam driver which is our use case for this API, could you test this patch series ? -- Regards, Laurent Pinchart
[PATCH] drm/sched: Add device pointer to drm_gpu_scheduler
Add device pointer so scheduler's printing can use DRM_DEV_ERROR() instead, which makes life easier under multiple GPU scenario. v2: amend all calls of drm_sched_init() v3: fill dev pointer for all drm_sched_init() calls Signed-off-by: Jiawei Gu --- drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_sched.c | 2 +- drivers/gpu/drm/lima/lima_sched.c | 2 +- drivers/gpu/drm/panfrost/panfrost_job.c | 2 +- drivers/gpu/drm/scheduler/sched_main.c| 9 + drivers/gpu/drm/v3d/v3d_sched.c | 10 +- include/drm/gpu_scheduler.h | 3 ++- 7 files changed, 16 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c index 45977a72b5dd..cd2d594d4ffc 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c @@ -502,7 +502,7 @@ int amdgpu_fence_driver_init_ring(struct amdgpu_ring *ring, r = drm_sched_init(&ring->sched, &amdgpu_sched_ops, num_hw_submission, amdgpu_job_hang_limit, - timeout, NULL, sched_score, ring->name); + timeout, NULL, sched_score, ring->name, adev->dev); if (r) { DRM_ERROR("Failed to create scheduler on ring %s.\n", ring->name); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c b/drivers/gpu/drm/etnaviv/etnaviv_sched.c index 58f593b278c1..35e5ef7dbdcc 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c @@ -195,7 +195,7 @@ int etnaviv_sched_init(struct etnaviv_gpu *gpu) ret = drm_sched_init(&gpu->sched, &etnaviv_sched_ops, etnaviv_hw_jobs_limit, etnaviv_job_hang_limit, msecs_to_jiffies(500), NULL, NULL, -dev_name(gpu->dev)); +dev_name(gpu->dev), gpu->dev); if (ret) return ret; diff --git a/drivers/gpu/drm/lima/lima_sched.c b/drivers/gpu/drm/lima/lima_sched.c index 5612d73f238f..8d517c8880e3 100644 --- a/drivers/gpu/drm/lima/lima_sched.c +++ b/drivers/gpu/drm/lima/lima_sched.c @@ -490,7 +490,7 @@ int lima_sched_pipe_init(struct lima_sched_pipe *pipe, const char *name) return drm_sched_init(&pipe->base, &lima_sched_ops, 1, lima_job_hang_limit, msecs_to_jiffies(timeout), NULL, - NULL, name); + NULL, name, pipe->ldev->dev); } void lima_sched_pipe_fini(struct lima_sched_pipe *pipe) diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 908d79520853..a6925dbb6224 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -812,7 +812,7 @@ int panfrost_job_init(struct panfrost_device *pfdev) nentries, 0, msecs_to_jiffies(JOB_TIMEOUT_MS), pfdev->reset.wq, -NULL, "pan_js"); +NULL, "pan_js", pfdev->dev); if (ret) { dev_err(pfdev->dev, "Failed to create scheduler: %d.", ret); goto err_sched; diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index f91fb31ab7a7..b81fceb0b8a2 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -491,7 +491,7 @@ void drm_sched_start(struct drm_gpu_scheduler *sched, bool full_recovery) if (r == -ENOENT) drm_sched_job_done(s_job); else if (r) - DRM_ERROR("fence add callback failed (%d)\n", + DRM_DEV_ERROR(sched->dev, "fence add callback failed (%d)\n", r); } else drm_sched_job_done(s_job); @@ -957,7 +957,7 @@ static int drm_sched_main(void *param) if (r == -ENOENT) drm_sched_job_done(sched_job); else if (r) - DRM_ERROR("fence add callback failed (%d)\n", + DRM_DEV_ERROR(sched->dev, "fence add callback failed (%d)\n", r); dma_fence_put(fence); } else { @@ -991,7 +991,7 @@ int drm_sched_init(struct drm_gpu_scheduler *sched, const struct drm_sched_backend_ops *ops, unsigned hw_submission, unsigned hang_limit, long timeout, struct workqueue_struct *timeout_wq, - atomic_t *score, const char *nam
[PATCH v2 00/22] drm: Fill in default values for plane properties
Hi, We have a bunch of functions that create planes properties that will take a default value, but it isn't actually enforced in the plane state. This leads to drivers having multiple strategies to work around that issue, most of them being a variation of forcing a value at plane state reset time. Others work fine by luck, or have entirely ignored the issue. This series aims at making sure the default value set by the call to the function isn't ignored, and then making sure all drivers behave consistently. Let me know what you think, Maxime Changes from v1: - Collected tags - Squashed some patches Dave Stevenson (3): drm/object: Add drm_object_property_get_default_value() function drm/object: Add default zpos value at reset drm/object: Add default color encoding and range value at reset Maxime Ripard (19): drm/komeda: plane: switch to plane reset helper drm/tegra: plane: switch to plane reset helper drm/tegra: hub: Fix zpos initial value mismatch drm/omap: plane: Fix zpos initial value mismatch drm/amd/display: Fix color encoding mismatch drm/tegra: plane: Remove redundant zpos initialisation drm/komeda: plane: Remove redundant zpos initialisation drm/exynos: plane: Remove redundant zpos initialisation drm/imx: ipuv3-plane: Remove redundant zpos initialisation drm/msm/mdp5: Remove redundant zpos initialisation drm/nouveau/kms: Remove redundant zpos initialisation drm/omap: plane: Remove redundant zpos initialisation drm/rcar: plane: Remove redundant zpos initialisation drm/sti: plane: Remove redundant zpos initialisation drm/sun4i: layer: Remove redundant zpos initialisation drm/komeda: plane: Remove redundant color encoding and range initialisation drm/armada: overlay: Remove redundant color encoding and range initialisation drm/imx: ipuv3-plane: Remove redundant color encoding and range initialisation drm/omap: plane: Remove redundant color encoding and range initialisation .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- .../gpu/drm/arm/display/komeda/komeda_plane.c | 13 + drivers/gpu/drm/armada/armada_overlay.c | 2 - drivers/gpu/drm/drm_atomic_state_helper.c | 25 + drivers/gpu/drm/drm_mode_object.c | 53 +++ drivers/gpu/drm/exynos/exynos_drm_plane.c | 5 +- drivers/gpu/drm/imx/ipuv3-plane.c | 8 +-- drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c| 16 +++--- drivers/gpu/drm/nouveau/dispnv50/wndw.c | 2 - drivers/gpu/drm/omapdrm/omap_plane.c | 22 drivers/gpu/drm/rcar-du/rcar_du_plane.c | 1 - drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 1 - drivers/gpu/drm/sti/sti_cursor.c | 2 +- drivers/gpu/drm/sti/sti_gdp.c | 2 +- drivers/gpu/drm/sti/sti_hqvdp.c | 2 +- drivers/gpu/drm/sti/sti_plane.c | 6 --- drivers/gpu/drm/sti/sti_plane.h | 1 - drivers/gpu/drm/sun4i/sun4i_layer.c | 16 +++--- drivers/gpu/drm/tegra/hub.c | 2 +- drivers/gpu/drm/tegra/plane.c | 6 +-- include/drm/drm_mode_object.h | 7 +++ 21 files changed, 111 insertions(+), 83 deletions(-) -- 2.35.1
[PATCH v2 01/22] drm/komeda: plane: switch to plane reset helper
komeda_plane_reset() does the state initialisation by copying a lot of the code found in the __drm_atomic_helper_plane_reset(). Let's switch to that helper and reduce the boilerplate. Cc: Brian Starkey Cc: "James (Qian) Wang" Cc: Liviu Dudau Cc: Mihail Atanassov Signed-off-by: Maxime Ripard --- drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c index d63d83800a8a..1778f6e1ea56 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c @@ -145,14 +145,10 @@ static void komeda_plane_reset(struct drm_plane *plane) state = kzalloc(sizeof(*state), GFP_KERNEL); if (state) { - state->base.rotation = DRM_MODE_ROTATE_0; - state->base.pixel_blend_mode = DRM_MODE_BLEND_PREMULTI; - state->base.alpha = DRM_BLEND_ALPHA_OPAQUE; + __drm_atomic_helper_plane_reset(plane, &state->base); state->base.zpos = kplane->layer->base.id; state->base.color_encoding = DRM_COLOR_YCBCR_BT601; state->base.color_range = DRM_COLOR_YCBCR_LIMITED_RANGE; - plane->state = &state->base; - plane->state->plane = plane; } } -- 2.35.1
[PATCH v2 02/22] drm/tegra: plane: switch to plane reset helper
tegra_plane_reset() does the state initialisation by copying a lot of the code found in the __drm_atomic_helper_plane_reset(). Let's switch to that helper and reduce the boilerplate. Cc: linux-te...@vger.kernel.org Cc: Jonathan Hunter Cc: Thierry Reding Signed-off-by: Maxime Ripard --- drivers/gpu/drm/tegra/plane.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c index 321cb1f13da6..ec0822c86926 100644 --- a/drivers/gpu/drm/tegra/plane.c +++ b/drivers/gpu/drm/tegra/plane.c @@ -37,8 +37,7 @@ static void tegra_plane_reset(struct drm_plane *plane) state = kzalloc(sizeof(*state), GFP_KERNEL); if (state) { - plane->state = &state->base; - plane->state->plane = plane; + __drm_atomic_helper_plane_reset(plane, &state->base); plane->state->zpos = p->index; plane->state->normalized_zpos = p->index; -- 2.35.1
[PATCH v2 05/22] drm/amd/display: Fix color encoding mismatch
The amdgpu KMS driver calls drm_plane_create_color_properties() with a default encoding set to BT709. However, the core will ignore it and the driver doesn't force it in its plane state reset hook, so the initial value will be 0, which represents BT601. Fix the mismatch by using an initial value of BT601 in drm_plane_create_color_properties(). Cc: amd-...@lists.freedesktop.org Cc: Alex Deucher Cc: "Christian König" Cc: Harry Wentland Cc: Leo Li Cc: "Pan, Xinhui" Cc: Rodrigo Siqueira Signed-off-by: Maxime Ripard --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index feccf2b555d2..86b27a355e90 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -7914,7 +7914,7 @@ static int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm, BIT(DRM_COLOR_YCBCR_BT2020), BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) | BIT(DRM_COLOR_YCBCR_FULL_RANGE), - DRM_COLOR_YCBCR_BT709, DRM_COLOR_YCBCR_LIMITED_RANGE); + DRM_COLOR_YCBCR_BT601, DRM_COLOR_YCBCR_LIMITED_RANGE); } supported_rotations = -- 2.35.1
[PATCH v2 03/22] drm/tegra: hub: Fix zpos initial value mismatch
While the tegra_shared_plane_create() function calls drm_plane_create_zpos_property() with an initial value of 0, tegra_plane_reset() will force it to the plane index. Fix the discrepancy by setting the initial zpos value to the plane index in the function call. Cc: linux-te...@vger.kernel.org Cc: Jonathan Hunter Cc: Thierry Reding Signed-off-by: Maxime Ripard --- drivers/gpu/drm/tegra/hub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c index b910155f80c4..7f68a142d922 100644 --- a/drivers/gpu/drm/tegra/hub.c +++ b/drivers/gpu/drm/tegra/hub.c @@ -788,7 +788,7 @@ struct drm_plane *tegra_shared_plane_create(struct drm_device *drm, } drm_plane_helper_add(p, &tegra_shared_plane_helper_funcs); - drm_plane_create_zpos_property(p, 0, 0, 255); + drm_plane_create_zpos_property(p, index, 0, 255); return p; } -- 2.35.1
[PATCH v2 10/22] drm/exynos: plane: Remove redundant zpos initialisation
The exynos KMS driver will call drm_plane_create_zpos_property() with an init value depending on the plane purpose. Since the initial value wasn't carried over in the state, the driver had to set it again in exynos_drm_plane_reset(). However, the helpers have been adjusted to set it properly at reset, so this is not needed anymore. Cc: linux-arm-ker...@lists.infradead.org Cc: linux-samsung-...@vger.kernel.org Cc: Alim Akhtar Cc: Inki Dae Cc: Joonyoung Shim Cc: Krzysztof Kozlowski Cc: Kyungmin Park Cc: Seung-Woo Kim Signed-off-by: Maxime Ripard --- drivers/gpu/drm/exynos/exynos_drm_plane.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c index df76bdee7dca..3615cf329e32 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c @@ -122,7 +122,6 @@ static void exynos_plane_mode_set(struct exynos_drm_plane_state *exynos_state) static void exynos_drm_plane_reset(struct drm_plane *plane) { - struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane); struct exynos_drm_plane_state *exynos_state; if (plane->state) { @@ -133,10 +132,8 @@ static void exynos_drm_plane_reset(struct drm_plane *plane) } exynos_state = kzalloc(sizeof(*exynos_state), GFP_KERNEL); - if (exynos_state) { + if (exynos_state) __drm_atomic_helper_plane_reset(plane, &exynos_state->base); - plane->state->zpos = exynos_plane->config->zpos; - } } static struct drm_plane_state * -- 2.35.1
[PATCH v2 08/22] drm/tegra: plane: Remove redundant zpos initialisation
The tegra KMS driver will call drm_plane_create_zpos_property() with an init value of the plane index. Since the initial value wasn't carried over in the state, the driver had to set it again in tegra_plane_reset(). However, the helpers have been adjusted to set it properly at reset, so this is not needed anymore. Cc: linux-te...@vger.kernel.org Cc: Jonathan Hunter Cc: Thierry Reding Signed-off-by: Maxime Ripard --- drivers/gpu/drm/tegra/plane.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c index ec0822c86926..a00913d064d3 100644 --- a/drivers/gpu/drm/tegra/plane.c +++ b/drivers/gpu/drm/tegra/plane.c @@ -25,7 +25,6 @@ static void tegra_plane_destroy(struct drm_plane *plane) static void tegra_plane_reset(struct drm_plane *plane) { - struct tegra_plane *p = to_tegra_plane(plane); struct tegra_plane_state *state; unsigned int i; @@ -38,8 +37,6 @@ static void tegra_plane_reset(struct drm_plane *plane) state = kzalloc(sizeof(*state), GFP_KERNEL); if (state) { __drm_atomic_helper_plane_reset(plane, &state->base); - plane->state->zpos = p->index; - plane->state->normalized_zpos = p->index; for (i = 0; i < 3; i++) state->iova[i] = DMA_MAPPING_ERROR; -- 2.35.1
[PATCH v2 04/22] drm/omap: plane: Fix zpos initial value mismatch
While the omap_plane_init() function calls drm_plane_create_zpos_property() with an initial value of 0, omap_plane_reset() will force it to another value depending on the plane type. Fix the discrepancy by setting the initial zpos value to the same value in the drm_plane_create_zpos_property() call. Reviewed-by: Tomi Valkeinen Signed-off-by: Maxime Ripard --- drivers/gpu/drm/omapdrm/omap_plane.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c index b35205c4e979..e67baf9a942c 100644 --- a/drivers/gpu/drm/omapdrm/omap_plane.c +++ b/drivers/gpu/drm/omapdrm/omap_plane.c @@ -533,6 +533,7 @@ struct drm_plane *omap_plane_init(struct drm_device *dev, unsigned int num_planes = dispc_get_num_ovls(priv->dispc); struct drm_plane *plane; struct omap_plane *omap_plane; + unsigned int zpos; int ret; u32 nformats; const u32 *formats; @@ -564,7 +565,16 @@ struct drm_plane *omap_plane_init(struct drm_device *dev, drm_plane_helper_add(plane, &omap_plane_helper_funcs); omap_plane_install_properties(plane, &plane->base); - drm_plane_create_zpos_property(plane, 0, 0, num_planes - 1); + + /* +* Set the zpos default depending on whether we are a primary or overlay +* plane. +*/ + if (plane->type == DRM_PLANE_TYPE_PRIMARY) + zpos = 0; + else + zpos = omap_plane->id; + drm_plane_create_zpos_property(plane, zpos, 0, num_planes - 1); drm_plane_create_alpha_property(plane); drm_plane_create_blend_mode_property(plane, BIT(DRM_MODE_BLEND_PREMULTI) | BIT(DRM_MODE_BLEND_COVERAGE)); -- 2.35.1
[PATCH v2 12/22] drm/msm/mdp5: Remove redundant zpos initialisation
The mdp KMS driver will call drm_plane_create_zpos_property() with an init value depending on the plane purpose. Since the initial value wasn't carried over in the state, the driver had to set it again in mdp5_plane_reset(). However, the helpers have been adjusted to set it properly at reset, so this is not needed anymore. Cc: freedr...@lists.freedesktop.org Cc: linux-arm-...@vger.kernel.org Cc: Abhinav Kumar Cc: Rob Clark Cc: Sean Paul Reviewed-by: Dmitry Baryshkov Signed-off-by: Maxime Ripard --- drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c index c6b69afcbac8..5d8ac84c510b 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c @@ -48,6 +48,8 @@ static void mdp5_plane_destroy(struct drm_plane *plane) static void mdp5_plane_install_properties(struct drm_plane *plane, struct drm_mode_object *obj) { + unsigned int zpos; + drm_plane_create_rotation_property(plane, DRM_MODE_ROTATE_0, DRM_MODE_ROTATE_0 | @@ -59,7 +61,12 @@ static void mdp5_plane_install_properties(struct drm_plane *plane, BIT(DRM_MODE_BLEND_PIXEL_NONE) | BIT(DRM_MODE_BLEND_PREMULTI) | BIT(DRM_MODE_BLEND_COVERAGE)); - drm_plane_create_zpos_property(plane, 1, 1, 255); + + if (plane->type == DRM_PLANE_TYPE_PRIMARY) + zpos = STAGE_BASE; + else + zpos = STAGE0 + drm_plane_index(plane); + drm_plane_create_zpos_property(plane, zpos, 1, 255); } static void @@ -91,13 +98,6 @@ static void mdp5_plane_reset(struct drm_plane *plane) kfree(to_mdp5_plane_state(plane->state)); mdp5_state = kzalloc(sizeof(*mdp5_state), GFP_KERNEL); - - if (plane->type == DRM_PLANE_TYPE_PRIMARY) - mdp5_state->base.zpos = STAGE_BASE; - else - mdp5_state->base.zpos = STAGE0 + drm_plane_index(plane); - mdp5_state->base.normalized_zpos = mdp5_state->base.zpos; - __drm_atomic_helper_plane_reset(plane, &mdp5_state->base); } -- 2.35.1
[PATCH v2 07/22] drm/object: Add default zpos value at reset
From: Dave Stevenson The drm_plane_create_zpos_property() function asks for an initial value, and will set the associated plane state variable with that value if a state is present. However, that function is usually called at a time where there's no state yet. Since the drm_plane_state reset helper doesn't take care of reading that value when it's called, it means that in most cases the initial value will be 0, or the drivers will have to work around it. Let's add some code in __drm_atomic_helper_plane_state_reset() to get the initial zpos value if the property has been attached in order to fix this. Reviewed-by: Harry Wentland Reviewed-by: Laurent Pinchart Signed-off-by: Dave Stevenson Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_atomic_state_helper.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c index ddcf5c2c8e6a..1412cee404ca 100644 --- a/drivers/gpu/drm/drm_atomic_state_helper.c +++ b/drivers/gpu/drm/drm_atomic_state_helper.c @@ -243,11 +243,22 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state); void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *plane_state, struct drm_plane *plane) { + u64 val; + plane_state->plane = plane; plane_state->rotation = DRM_MODE_ROTATE_0; plane_state->alpha = DRM_BLEND_ALPHA_OPAQUE; plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI; + + if (plane->zpos_property) { + if (!drm_object_property_get_default_value(&plane->base, + plane->zpos_property, + &val)) { + plane_state->zpos = val; + plane_state->normalized_zpos = val; + } + } } EXPORT_SYMBOL(__drm_atomic_helper_plane_state_reset); -- 2.35.1
[PATCH v2 15/22] drm/rcar: plane: Remove redundant zpos initialisation
The rcar-du KMS driver will call drm_plane_create_zpos_property() with an init value depending on the plane type. Since the initial value wasn't carried over in the state, the driver had to set it again in rcar_du_plane_reset() and rcar_du_vsp_plane_reset(). However, the helpers have been adjusted to set it properly at reset, so this is not needed anymore. Cc: linux-renesas-...@vger.kernel.org Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart Signed-off-by: Maxime Ripard --- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 1 - drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 1 - 2 files changed, 2 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index 862197be1e01..9dda5e06457d 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c @@ -696,7 +696,6 @@ static void rcar_du_plane_reset(struct drm_plane *plane) state->hwindex = -1; state->source = RCAR_DU_PLANE_MEMORY; state->colorkey = RCAR_DU_COLORKEY_NONE; - state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1; } static int rcar_du_plane_atomic_set_property(struct drm_plane *plane, diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index b7fc5b069cbc..719c60034952 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c @@ -362,7 +362,6 @@ static void rcar_du_vsp_plane_reset(struct drm_plane *plane) return; __drm_atomic_helper_plane_reset(plane, &state->state); - state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1; } static const struct drm_plane_funcs rcar_du_vsp_plane_funcs = { -- 2.35.1
[PATCH v2 14/22] drm/omap: plane: Remove redundant zpos initialisation
The omap KMS driver will call drm_plane_create_zpos_property() with an init value of the plane index and the plane type. Since the initial value wasn't carried over in the state, the driver had to set it again in omap_plane_reset(). However, the helpers have been adjusted to set it properly at reset, so this is not needed anymore. Reviewed-by: Tomi Valkeinen Signed-off-by: Maxime Ripard --- drivers/gpu/drm/omapdrm/omap_plane.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c index e67baf9a942c..d96bc929072a 100644 --- a/drivers/gpu/drm/omapdrm/omap_plane.c +++ b/drivers/gpu/drm/omapdrm/omap_plane.c @@ -414,13 +414,6 @@ static void omap_plane_reset(struct drm_plane *plane) return; __drm_atomic_helper_plane_reset(plane, &omap_state->base); - - /* -* Set the zpos default depending on whether we are a primary or overlay -* plane. -*/ - plane->state->zpos = plane->type == DRM_PLANE_TYPE_PRIMARY - ? 0 : omap_plane->id; plane->state->color_encoding = DRM_COLOR_YCBCR_BT601; plane->state->color_range = DRM_COLOR_YCBCR_FULL_RANGE; } -- 2.35.1
[PATCH v2 09/22] drm/komeda: plane: Remove redundant zpos initialisation
The komeda KMS driver will call drm_plane_create_zpos_property() with an init value of the plane index. Since the initial value wasn't carried over in the state, the driver had to set it again in komeda_plane_reset(). However, the helpers have been adjusted to set it properly at reset, so this is not needed anymore. Cc: Brian Starkey Cc: "James (Qian) Wang" Cc: Liviu Dudau Cc: Mihail Atanassov Signed-off-by: Maxime Ripard --- drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c index 1778f6e1ea56..383bb2039bd4 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c @@ -135,7 +135,6 @@ static void komeda_plane_destroy(struct drm_plane *plane) static void komeda_plane_reset(struct drm_plane *plane) { struct komeda_plane_state *state; - struct komeda_plane *kplane = to_kplane(plane); if (plane->state) __drm_atomic_helper_plane_destroy_state(plane->state); @@ -146,7 +145,6 @@ static void komeda_plane_reset(struct drm_plane *plane) state = kzalloc(sizeof(*state), GFP_KERNEL); if (state) { __drm_atomic_helper_plane_reset(plane, &state->base); - state->base.zpos = kplane->layer->base.id; state->base.color_encoding = DRM_COLOR_YCBCR_BT601; state->base.color_range = DRM_COLOR_YCBCR_LIMITED_RANGE; } -- 2.35.1
[PATCH v2 17/22] drm/sun4i: layer: Remove redundant zpos initialisation
The sun4i KMS driver will call drm_plane_create_zpos_property() with an init value depending on the plane type. Since the initial value wasn't carried over in the state, the driver had to set it again in sun4i_backend_layer_reset(). However, the helpers have been adjusted to set it properly at reset, so this is not needed anymore. Cc: linux-arm-ker...@lists.infradead.org Cc: linux-su...@lists.linux.dev Cc: Chen-Yu Tsai Reviewed-by: Jernej Skrabec Signed-off-by: Maxime Ripard --- drivers/gpu/drm/sun4i/sun4i_layer.c | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c index 929e95f86b5b..6d43080791a0 100644 --- a/drivers/gpu/drm/sun4i/sun4i_layer.c +++ b/drivers/gpu/drm/sun4i/sun4i_layer.c @@ -18,7 +18,6 @@ static void sun4i_backend_layer_reset(struct drm_plane *plane) { - struct sun4i_layer *layer = plane_to_sun4i_layer(plane); struct sun4i_layer_state *state; if (plane->state) { @@ -31,10 +30,8 @@ static void sun4i_backend_layer_reset(struct drm_plane *plane) } state = kzalloc(sizeof(*state), GFP_KERNEL); - if (state) { + if (state) __drm_atomic_helper_plane_reset(plane, &state->state); - plane->state->zpos = layer->id; - } } static struct drm_plane_state * @@ -192,7 +189,8 @@ static const uint64_t sun4i_layer_modifiers[] = { static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm, struct sun4i_backend *backend, - enum drm_plane_type type) + enum drm_plane_type type, + unsigned int id) { const uint64_t *modifiers = sun4i_layer_modifiers; const uint32_t *formats = sun4i_layer_formats; @@ -204,6 +202,7 @@ static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm, if (!layer) return ERR_PTR(-ENOMEM); + layer->id = id; layer->backend = backend; if (IS_ERR_OR_NULL(backend->frontend)) { @@ -226,8 +225,8 @@ static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm, &sun4i_backend_layer_helper_funcs); drm_plane_create_alpha_property(&layer->plane); - drm_plane_create_zpos_property(&layer->plane, 0, 0, - SUN4I_BACKEND_NUM_LAYERS - 1); + drm_plane_create_zpos_property(&layer->plane, layer->id, + 0, SUN4I_BACKEND_NUM_LAYERS - 1); return layer; } @@ -249,14 +248,13 @@ struct drm_plane **sun4i_layers_init(struct drm_device *drm, enum drm_plane_type type = i ? DRM_PLANE_TYPE_OVERLAY : DRM_PLANE_TYPE_PRIMARY; struct sun4i_layer *layer; - layer = sun4i_layer_init_one(drm, backend, type); + layer = sun4i_layer_init_one(drm, backend, type, i); if (IS_ERR(layer)) { dev_err(drm->dev, "Couldn't initialize %s plane\n", i ? "overlay" : "primary"); return ERR_CAST(layer); } - layer->id = i; planes[i] = &layer->plane; } -- 2.35.1
[PATCH v2 16/22] drm/sti: plane: Remove redundant zpos initialisation
The sti KMS driver will call drm_plane_create_zpos_property() with an init value depending on the plane type. Since the initial value wasn't carried over in the state, the driver had to set it again in sti_plane_reset(). However, the helpers have been adjusted to set it properly at reset, so this is not needed anymore. Reviewed-by: Alain Volmat Reviewed-by: Philippe Cornu Signed-off-by: Maxime Ripard --- drivers/gpu/drm/sti/sti_cursor.c | 2 +- drivers/gpu/drm/sti/sti_gdp.c| 2 +- drivers/gpu/drm/sti/sti_hqvdp.c | 2 +- drivers/gpu/drm/sti/sti_plane.c | 6 -- drivers/gpu/drm/sti/sti_plane.h | 1 - 5 files changed, 3 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/sti/sti_cursor.c b/drivers/gpu/drm/sti/sti_cursor.c index 1d6051b4f6fe..414c9973aa6d 100644 --- a/drivers/gpu/drm/sti/sti_cursor.c +++ b/drivers/gpu/drm/sti/sti_cursor.c @@ -351,7 +351,7 @@ static const struct drm_plane_funcs sti_cursor_plane_helpers_funcs = { .update_plane = drm_atomic_helper_update_plane, .disable_plane = drm_atomic_helper_disable_plane, .destroy = drm_plane_cleanup, - .reset = sti_plane_reset, + .reset = drm_atomic_helper_plane_reset, .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state, .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, .late_register = sti_cursor_late_register, diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c index d1a35d97bc45..3db3768a3241 100644 --- a/drivers/gpu/drm/sti/sti_gdp.c +++ b/drivers/gpu/drm/sti/sti_gdp.c @@ -905,7 +905,7 @@ static const struct drm_plane_funcs sti_gdp_plane_helpers_funcs = { .update_plane = drm_atomic_helper_update_plane, .disable_plane = drm_atomic_helper_disable_plane, .destroy = drm_plane_cleanup, - .reset = sti_plane_reset, + .reset = drm_atomic_helper_plane_reset, .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state, .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, .late_register = sti_gdp_late_register, diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c index 3c61ba8b43e0..2201a50353eb 100644 --- a/drivers/gpu/drm/sti/sti_hqvdp.c +++ b/drivers/gpu/drm/sti/sti_hqvdp.c @@ -1283,7 +1283,7 @@ static const struct drm_plane_funcs sti_hqvdp_plane_helpers_funcs = { .update_plane = drm_atomic_helper_update_plane, .disable_plane = drm_atomic_helper_disable_plane, .destroy = drm_plane_cleanup, - .reset = sti_plane_reset, + .reset = drm_atomic_helper_plane_reset, .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state, .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, .late_register = sti_hqvdp_late_register, diff --git a/drivers/gpu/drm/sti/sti_plane.c b/drivers/gpu/drm/sti/sti_plane.c index 3da4a46df2f2..173409cdb99e 100644 --- a/drivers/gpu/drm/sti/sti_plane.c +++ b/drivers/gpu/drm/sti/sti_plane.c @@ -112,12 +112,6 @@ static int sti_plane_get_default_zpos(enum drm_plane_type type) return 0; } -void sti_plane_reset(struct drm_plane *plane) -{ - drm_atomic_helper_plane_reset(plane); - plane->state->zpos = sti_plane_get_default_zpos(plane->type); -} - static void sti_plane_attach_zorder_property(struct drm_plane *drm_plane, enum drm_plane_type type) { diff --git a/drivers/gpu/drm/sti/sti_plane.h b/drivers/gpu/drm/sti/sti_plane.h index 065bfb4a..8e33e629d9b0 100644 --- a/drivers/gpu/drm/sti/sti_plane.h +++ b/drivers/gpu/drm/sti/sti_plane.h @@ -81,5 +81,4 @@ void sti_plane_update_fps(struct sti_plane *plane, void sti_plane_init_property(struct sti_plane *plane, enum drm_plane_type type); -void sti_plane_reset(struct drm_plane *plane); #endif -- 2.35.1
[PATCH v2 06/22] drm/object: Add drm_object_property_get_default_value() function
From: Dave Stevenson Some functions to create properties (drm_plane_create_zpos_property or drm_plane_create_color_properties for example) will ask for a range of acceptable value and an initial one. This initial value is then stored in the values array for that property. Let's provide an helper to access this property. Reviewed-by: Laurent Pinchart Signed-off-by: Dave Stevenson Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_mode_object.c | 53 +-- include/drm/drm_mode_object.h | 7 2 files changed, 50 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c index 86d9e907c0b2..ba1608effc0f 100644 --- a/drivers/gpu/drm/drm_mode_object.c +++ b/drivers/gpu/drm/drm_mode_object.c @@ -297,11 +297,26 @@ int drm_object_property_set_value(struct drm_mode_object *obj, } EXPORT_SYMBOL(drm_object_property_set_value); +static int __drm_object_property_get_prop_value(struct drm_mode_object *obj, + struct drm_property *property, + uint64_t *val) +{ + int i; + + for (i = 0; i < obj->properties->count; i++) { + if (obj->properties->properties[i] == property) { + *val = obj->properties->values[i]; + return 0; + } + } + + return -EINVAL; +} + static int __drm_object_property_get_value(struct drm_mode_object *obj, struct drm_property *property, uint64_t *val) { - int i; /* read-only properties bypass atomic mechanism and still store * their value in obj->properties->values[].. mostly to avoid @@ -311,15 +326,7 @@ static int __drm_object_property_get_value(struct drm_mode_object *obj, !(property->flags & DRM_MODE_PROP_IMMUTABLE)) return drm_atomic_get_property(obj, property, val); - for (i = 0; i < obj->properties->count; i++) { - if (obj->properties->properties[i] == property) { - *val = obj->properties->values[i]; - return 0; - } - - } - - return -EINVAL; + return __drm_object_property_get_prop_value(obj, property, val); } /** @@ -348,6 +355,32 @@ int drm_object_property_get_value(struct drm_mode_object *obj, } EXPORT_SYMBOL(drm_object_property_get_value); +/** + * drm_object_property_get_default_value - retrieve the default value of a + * property when in atomic mode. + * @obj: drm mode object to get property value from + * @property: property to retrieve + * @val: storage for the property value + * + * This function retrieves the default state of the given property as passed in + * to drm_object_attach_property + * + * Only atomic drivers should call this function directly, as for non-atomic + * drivers it will return the current value. + * + * Returns: + * Zero on success, error code on failure. + */ +int drm_object_property_get_default_value(struct drm_mode_object *obj, + struct drm_property *property, + uint64_t *val) +{ + WARN_ON(!drm_drv_uses_atomic_modeset(property->dev)); + + return __drm_object_property_get_prop_value(obj, property, val); +} +EXPORT_SYMBOL(drm_object_property_get_default_value); + /* helper for getconnector and getproperties ioctls */ int drm_mode_object_get_properties(struct drm_mode_object *obj, bool atomic, uint32_t __user *prop_ptr, diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h index c34a3e8030e1..912f1e415685 100644 --- a/include/drm/drm_mode_object.h +++ b/include/drm/drm_mode_object.h @@ -98,6 +98,10 @@ struct drm_object_properties { * Hence atomic drivers should not use drm_object_property_set_value() * and drm_object_property_get_value() on mutable objects, i.e. those * without the DRM_MODE_PROP_IMMUTABLE flag set. +* +* For atomic drivers the default value of properties is stored in this +* array, so drm_object_property_get_default_value can be used to +* retrieve it. */ uint64_t values[DRM_OBJECT_MAX_PROPERTY]; }; @@ -126,6 +130,9 @@ int drm_object_property_set_value(struct drm_mode_object *obj, int drm_object_property_get_value(struct drm_mode_object *obj, struct drm_property *property, uint64_t *value); +int drm_object_property_get_default_value(struct drm_mode_object *obj, + struct drm_property *property, + uint64_t *val); void drm_object_attach_property(struct drm_mode_object *obj, struct drm_property *property, --
[PATCH v2 20/22] drm/armada: overlay: Remove redundant color encoding and range initialisation
The armada KMS driver will call drm_plane_create_color_properties() with a default encoding and range values of BT601 and Limited Range, respectively. Since the initial value wasn't carried over in the state, the driver had to set it again in armada_overlay_reset(). However, the helpers have been adjusted to set it properly at reset, so this is not needed anymore. Cc: Russell King Signed-off-by: Maxime Ripard --- drivers/gpu/drm/armada/armada_overlay.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index 424250535fed..a25539039efd 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -325,8 +325,6 @@ static void armada_overlay_reset(struct drm_plane *plane) state->contrast = DEFAULT_CONTRAST; state->saturation = DEFAULT_SATURATION; __drm_atomic_helper_plane_reset(plane, &state->base.base); - state->base.base.color_encoding = DEFAULT_ENCODING; - state->base.base.color_range = DRM_COLOR_YCBCR_LIMITED_RANGE; } } -- 2.35.1
[PATCH v2 18/22] drm/object: Add default color encoding and range value at reset
From: Dave Stevenson The drm_plane_create_color_properties() function asks for an initial value for the color encoding and range, and will set the associated plane state variable with that value if a state is present. However, that function is usually called at a time where there's no state yet. Since the drm_plane_state reset helper doesn't take care of reading that value when it's called, it means that in most cases the initial value will be 0 (so DRM_COLOR_YCBCR_BT601 and DRM_COLOR_YCBCR_LIMITED_RANGE, respectively), or the drivers will have to work around it. Let's add some code in __drm_atomic_helper_plane_state_reset() to get the initial encoding and range value if the property has been attached in order to fix this. Reviewed-by: Harry Wentland Signed-off-by: Dave Stevenson Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_atomic_state_helper.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c index 1412cee404ca..3b6d3bdbd099 100644 --- a/drivers/gpu/drm/drm_atomic_state_helper.c +++ b/drivers/gpu/drm/drm_atomic_state_helper.c @@ -251,6 +251,20 @@ void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *plane_state, plane_state->alpha = DRM_BLEND_ALPHA_OPAQUE; plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI; + if (plane->color_encoding_property) { + if (!drm_object_property_get_default_value(&plane->base, + plane->color_encoding_property, + &val)) + plane_state->color_encoding = val; + } + + if (plane->color_range_property) { + if (!drm_object_property_get_default_value(&plane->base, + plane->color_range_property, + &val)) + plane_state->color_range = val; + } + if (plane->zpos_property) { if (!drm_object_property_get_default_value(&plane->base, plane->zpos_property, -- 2.35.1
[PATCH v2 13/22] drm/nouveau/kms: Remove redundant zpos initialisation
The nouveau KMS driver will call drm_plane_create_zpos_property() with an init value depending on the plane purpose. Since the initial value wasn't carried over in the state, the driver had to set it again in nv50_wndw_reset(). However, the helpers have been adjusted to set it properly at reset, so this is not needed anymore. Cc: nouv...@lists.freedesktop.org Cc: Ben Skeggs Cc: Karol Herbst Cc: Lyude Paul Signed-off-by: Maxime Ripard --- drivers/gpu/drm/nouveau/dispnv50/wndw.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c index 133c8736426a..0c1a2ea0ed04 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c @@ -635,8 +635,6 @@ nv50_wndw_reset(struct drm_plane *plane) plane->funcs->atomic_destroy_state(plane, plane->state); __drm_atomic_helper_plane_reset(plane, &asyw->state); - plane->state->zpos = nv50_wndw_zpos_default(plane); - plane->state->normalized_zpos = nv50_wndw_zpos_default(plane); } static void -- 2.35.1
[PATCH v2 11/22] drm/imx: ipuv3-plane: Remove redundant zpos initialisation
The imx KMS driver will call drm_plane_create_zpos_property() with an init value depending on the plane purpose. Since the initial value wasn't carried over in the state, the driver had to set it again in ipu_plane_state_reset(). However, the helpers have been adjusted to set it properly at reset, so this is not needed anymore. Cc: linux-arm-ker...@lists.infradead.org Cc: NXP Linux Team Cc: Fabio Estevam Cc: Pengutronix Kernel Team Cc: Philipp Zabel Cc: Sascha Hauer Cc: Shawn Guo Signed-off-by: Maxime Ripard --- drivers/gpu/drm/imx/ipuv3-plane.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c index 846c1aae69c8..414bdf08d0b0 100644 --- a/drivers/gpu/drm/imx/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3-plane.c @@ -297,7 +297,6 @@ void ipu_plane_disable_deferred(struct drm_plane *plane) static void ipu_plane_state_reset(struct drm_plane *plane) { - unsigned int zpos = (plane->type == DRM_PLANE_TYPE_PRIMARY) ? 0 : 1; struct ipu_plane_state *ipu_state; if (plane->state) { @@ -311,8 +310,6 @@ static void ipu_plane_state_reset(struct drm_plane *plane) if (ipu_state) { __drm_atomic_helper_plane_reset(plane, &ipu_state->base); - ipu_state->base.zpos = zpos; - ipu_state->base.normalized_zpos = zpos; ipu_state->base.color_encoding = DRM_COLOR_YCBCR_BT601; ipu_state->base.color_range = DRM_COLOR_YCBCR_LIMITED_RANGE; } -- 2.35.1
[PATCH v2 19/22] drm/komeda: plane: Remove redundant color encoding and range initialisation
The komeda KMS driver will call drm_plane_create_color_properties() with a default encoding and range values of BT601 and Limited Range, respectively. Since the initial value wasn't carried over in the state, the driver had to set it again in komeda_plane_reset(). However, the helpers have been adjusted to set it properly at reset, so this is not needed anymore. Cc: Brian Starkey Cc: "James (Qian) Wang" Cc: Liviu Dudau Cc: Mihail Atanassov Signed-off-by: Maxime Ripard --- drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c index 383bb2039bd4..541949f2d44a 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c @@ -143,11 +143,8 @@ static void komeda_plane_reset(struct drm_plane *plane) plane->state = NULL; state = kzalloc(sizeof(*state), GFP_KERNEL); - if (state) { + if (state) __drm_atomic_helper_plane_reset(plane, &state->base); - state->base.color_encoding = DRM_COLOR_YCBCR_BT601; - state->base.color_range = DRM_COLOR_YCBCR_LIMITED_RANGE; - } } static struct drm_plane_state * -- 2.35.1
[PATCH v2 22/22] drm/omap: plane: Remove redundant color encoding and range initialisation
The omap KMS driver will call drm_plane_create_color_properties() with a default encoding and range values of BT601 and Full Range, respectively. Since the initial value wasn't carried over in the state, the driver had to set it again in omap_plane_reset(). However, the helpers have been adjusted to set it properly at reset, so this is not needed anymore. Reviewed-by: Tomi Valkeinen Signed-off-by: Maxime Ripard --- drivers/gpu/drm/omapdrm/omap_plane.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c index d96bc929072a..b83d91ec030a 100644 --- a/drivers/gpu/drm/omapdrm/omap_plane.c +++ b/drivers/gpu/drm/omapdrm/omap_plane.c @@ -403,7 +403,6 @@ void omap_plane_install_properties(struct drm_plane *plane, static void omap_plane_reset(struct drm_plane *plane) { - struct omap_plane *omap_plane = to_omap_plane(plane); struct omap_plane_state *omap_state; if (plane->state) @@ -414,8 +413,6 @@ static void omap_plane_reset(struct drm_plane *plane) return; __drm_atomic_helper_plane_reset(plane, &omap_state->base); - plane->state->color_encoding = DRM_COLOR_YCBCR_BT601; - plane->state->color_range = DRM_COLOR_YCBCR_FULL_RANGE; } static struct drm_plane_state * -- 2.35.1
[PATCH v2 21/22] drm/imx: ipuv3-plane: Remove redundant color encoding and range initialisation
The imx KMS driver will call drm_plane_create_color_properties() with a default encoding and range values of BT601 and Limited Range, respectively. Since the initial value wasn't carried over in the state, the driver had to set it again in ipu_plane_state_reset(). However, the helpers have been adjusted to set it properly at reset, so this is not needed anymore. Cc: linux-arm-ker...@lists.infradead.org Cc: NXP Linux Team Cc: Fabio Estevam Cc: Pengutronix Kernel Team Cc: Philipp Zabel Cc: Sascha Hauer Cc: Shawn Guo Signed-off-by: Maxime Ripard --- drivers/gpu/drm/imx/ipuv3-plane.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c index 414bdf08d0b0..36b32e8806e3 100644 --- a/drivers/gpu/drm/imx/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3-plane.c @@ -308,11 +308,8 @@ static void ipu_plane_state_reset(struct drm_plane *plane) ipu_state = kzalloc(sizeof(*ipu_state), GFP_KERNEL); - if (ipu_state) { + if (ipu_state) __drm_atomic_helper_plane_reset(plane, &ipu_state->base); - ipu_state->base.color_encoding = DRM_COLOR_YCBCR_BT601; - ipu_state->base.color_range = DRM_COLOR_YCBCR_LIMITED_RANGE; - } } static struct drm_plane_state * -- 2.35.1
Re: [PATCH v10 1/4] MIPS: Loongson64: dts: update the display controller device node
On 21/02/2022 10:19, Sergei Shtylyov wrote: > On 2/20/22 5:55 PM, Sui Jingfeng wrote: > >> From: suijingfeng >> >> The display controller is a pci device, its PCI vendor id is 0x0014 >> its PCI device id is 0x7a06. >> >> 1) In order to let the driver to know which chip the DC is contained >>in, the compatible string of the display controller is updated >>according to the chip's name. >> >> 2) Add display controller device node for ls2k1000 SoC >> >> Reported-by: Krzysztof Kozlowski >> Signed-off-by: suijingfeng >> Signed-off-by: Sui Jingfeng <15330273...@189.cn> >> --- >> arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi | 8 >> arch/mips/boot/dts/loongson/ls7a-pch.dtsi | 7 ++- >> 2 files changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi >> b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi >> index 768cf2abcea3..af9cda540f9e 100644 >> --- a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi >> +++ b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi >> @@ -209,6 +209,14 @@ gpu@5,0 { >> interrupt-parent = <&liointc0>; >> }; >> >> +lsdc: display-controller@6,0 { > >Shouldn't the node name just be "display", according to the section 2.2.2 > of the DT spec? lcd-controller, led-controller. As I understood from the bindings, this is not physical device displaying something (like a panel) but rather a device controlling such panel. Therefore display-controller feels appropriate. Best regards, Krzysztof
[PATCH] drm/amdgpu: check vm ready by evicting
Workstation application ANSA/META get this error dmesg: [drm:amdgpu_gem_va_ioctl [amdgpu]] *ERROR* Couldn't update BO_VA (-16) This is caused by: 1. create a 256MB buffer in invisible VRAM 2. CPU map the buffer and access it causes vm_fault and try to move it to visible VRAM 3. force visible VRAM space and traverse all VRAM bos to check if evicting this bo is valuable 4. when checking a VM bo (in invisible VRAM), amdgpu_vm_evictable() will set amdgpu_vm->evicting, but latter due to not in visible VRAM, won't really evict it so not add it to amdgpu_vm->evicted 5. before next CS to clear the amdgpu_vm->evicting, user VM ops ioctl will pass amdgpu_vm_ready() (check amdgpu_vm->evicted) but fail in amdgpu_vm_bo_update_mapping() (check amdgpu_vm->evicting) and get this error log This error won't affect functionality as next CS will finish the waiting VM ops. But we'd better clear the error log by check the evicting flag which really stop VM ops latter. Signed-off-by: Qiang Yu --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 37acd8911168..2cd9f1a2e5fa 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -770,11 +770,16 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm, * Check if all VM PDs/PTs are ready for updates * * Returns: - * True if eviction list is empty. + * True if VM is not evicting. */ bool amdgpu_vm_ready(struct amdgpu_vm *vm) { - return list_empty(&vm->evicted); + bool ret; + + amdgpu_vm_eviction_lock(vm); + ret = !vm->evicting; + amdgpu_vm_eviction_unlock(vm); + return ret; } /** -- 2.25.1
Re: (subset) [Intel-gfx] [PATCH 17/22] drm/vc4: Use drm_mode_copy()
On Fri, 18 Feb 2022 12:03:58 +0200, Ville Syrjala wrote: > From: Ville Syrjälä > > struct drm_display_mode embeds a list head, so overwriting > the full struct with another one will corrupt the list > (if the destination mode is on a list). Use drm_mode_copy() > instead which explicitly preserves the list head of > the destination mode. > > [...] Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
Re: [PATCH] drm/sched: Add device pointer to drm_gpu_scheduler
Am 21.02.22 um 10:57 schrieb Jiawei Gu: Add device pointer so scheduler's printing can use DRM_DEV_ERROR() instead, which makes life easier under multiple GPU scenario. v2: amend all calls of drm_sched_init() v3: fill dev pointer for all drm_sched_init() calls Signed-off-by: Jiawei Gu Reviewed-by: Christian König When Andrey is fine with that as well I think the best approach is to push this upstream through drm-misc-next since it touches multiple drivers. We can merge it into the DKMS branch as well if necessary. Regards, Christian --- drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_sched.c | 2 +- drivers/gpu/drm/lima/lima_sched.c | 2 +- drivers/gpu/drm/panfrost/panfrost_job.c | 2 +- drivers/gpu/drm/scheduler/sched_main.c| 9 + drivers/gpu/drm/v3d/v3d_sched.c | 10 +- include/drm/gpu_scheduler.h | 3 ++- 7 files changed, 16 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c index 45977a72b5dd..cd2d594d4ffc 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c @@ -502,7 +502,7 @@ int amdgpu_fence_driver_init_ring(struct amdgpu_ring *ring, r = drm_sched_init(&ring->sched, &amdgpu_sched_ops, num_hw_submission, amdgpu_job_hang_limit, - timeout, NULL, sched_score, ring->name); + timeout, NULL, sched_score, ring->name, adev->dev); if (r) { DRM_ERROR("Failed to create scheduler on ring %s.\n", ring->name); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c b/drivers/gpu/drm/etnaviv/etnaviv_sched.c index 58f593b278c1..35e5ef7dbdcc 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c @@ -195,7 +195,7 @@ int etnaviv_sched_init(struct etnaviv_gpu *gpu) ret = drm_sched_init(&gpu->sched, &etnaviv_sched_ops, etnaviv_hw_jobs_limit, etnaviv_job_hang_limit, msecs_to_jiffies(500), NULL, NULL, -dev_name(gpu->dev)); +dev_name(gpu->dev), gpu->dev); if (ret) return ret; diff --git a/drivers/gpu/drm/lima/lima_sched.c b/drivers/gpu/drm/lima/lima_sched.c index 5612d73f238f..8d517c8880e3 100644 --- a/drivers/gpu/drm/lima/lima_sched.c +++ b/drivers/gpu/drm/lima/lima_sched.c @@ -490,7 +490,7 @@ int lima_sched_pipe_init(struct lima_sched_pipe *pipe, const char *name) return drm_sched_init(&pipe->base, &lima_sched_ops, 1, lima_job_hang_limit, msecs_to_jiffies(timeout), NULL, - NULL, name); + NULL, name, pipe->ldev->dev); } void lima_sched_pipe_fini(struct lima_sched_pipe *pipe) diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 908d79520853..a6925dbb6224 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -812,7 +812,7 @@ int panfrost_job_init(struct panfrost_device *pfdev) nentries, 0, msecs_to_jiffies(JOB_TIMEOUT_MS), pfdev->reset.wq, -NULL, "pan_js"); +NULL, "pan_js", pfdev->dev); if (ret) { dev_err(pfdev->dev, "Failed to create scheduler: %d.", ret); goto err_sched; diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index f91fb31ab7a7..b81fceb0b8a2 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -491,7 +491,7 @@ void drm_sched_start(struct drm_gpu_scheduler *sched, bool full_recovery) if (r == -ENOENT) drm_sched_job_done(s_job); else if (r) - DRM_ERROR("fence add callback failed (%d)\n", + DRM_DEV_ERROR(sched->dev, "fence add callback failed (%d)\n", r); } else drm_sched_job_done(s_job); @@ -957,7 +957,7 @@ static int drm_sched_main(void *param) if (r == -ENOENT) drm_sched_job_done(sched_job); else if (r) - DRM_ERROR("fence add callback failed (%d)\n", + DRM_DEV_ERROR(sched->dev, "fence add callback failed (%d)\n", r); dma_fence_put(fence); } else { @@
Re: [PATCH] drm/amdgpu: check vm ready by evicting
Am 21.02.22 um 11:12 schrieb Qiang Yu: Workstation application ANSA/META get this error dmesg: [drm:amdgpu_gem_va_ioctl [amdgpu]] *ERROR* Couldn't update BO_VA (-16) This is caused by: 1. create a 256MB buffer in invisible VRAM 2. CPU map the buffer and access it causes vm_fault and try to move it to visible VRAM 3. force visible VRAM space and traverse all VRAM bos to check if evicting this bo is valuable 4. when checking a VM bo (in invisible VRAM), amdgpu_vm_evictable() will set amdgpu_vm->evicting, but latter due to not in visible VRAM, won't really evict it so not add it to amdgpu_vm->evicted 5. before next CS to clear the amdgpu_vm->evicting, user VM ops ioctl will pass amdgpu_vm_ready() (check amdgpu_vm->evicted) but fail in amdgpu_vm_bo_update_mapping() (check amdgpu_vm->evicting) and get this error log This error won't affect functionality as next CS will finish the waiting VM ops. But we'd better clear the error log by check the evicting flag which really stop VM ops latter. Signed-off-by: Qiang Yu Reviewed-by: Christian König Good work. --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 37acd8911168..2cd9f1a2e5fa 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -770,11 +770,16 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm, * Check if all VM PDs/PTs are ready for updates * * Returns: - * True if eviction list is empty. + * True if VM is not evicting. */ bool amdgpu_vm_ready(struct amdgpu_vm *vm) { - return list_empty(&vm->evicted); + bool ret; + + amdgpu_vm_eviction_lock(vm); + ret = !vm->evicting; + amdgpu_vm_eviction_unlock(vm); + return ret; } /**
Re: [PATCH] drm: Only select I2C_ALGOBIT for drivers that actually need it
Hello, On Fri, May 14, 2021 at 12:01:42PM +0200, Uwe Kleine-König wrote: > While working on a drm driver that doesn't need the i2c algobit stuff I > noticed that DRM selects this code even tough only 8 drivers actually use > it. While also only some drivers use i2c, keep the select for I2C for the > next cleanup patch. Still prepare this already by also selecting I2C for > the individual drivers. > > Signed-off-by: Uwe Kleine-König This patch is already old but the issue is still valid and the patch even still applies to today's Linus' master branch. I didn't receive any human feedback. If you consider this patch worthwile I can recheck if it's still correct as is or needs adaption. Best regards Uwe > --- > drivers/gpu/drm/Kconfig | 5 - > drivers/gpu/drm/ast/Kconfig | 2 ++ > drivers/gpu/drm/gma500/Kconfig | 2 ++ > drivers/gpu/drm/hisilicon/hibmc/Kconfig | 2 ++ > drivers/gpu/drm/i915/Kconfig| 2 ++ > drivers/gpu/drm/mgag200/Kconfig | 2 ++ > drivers/gpu/drm/nouveau/Kconfig | 2 ++ > 7 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 3c16bd1afd87..351ea617c498 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -12,7 +12,6 @@ menuconfig DRM > select HDMI > select FB_CMDLINE > select I2C > - select I2C_ALGOBIT > select DMA_SHARED_BUFFER > select SYNC_FILE > # gallium uses SYS_kcmp for os_same_file_description() to de-duplicate > @@ -233,6 +232,8 @@ config DRM_RADEON > select DRM_KMS_HELPER > select DRM_TTM > select DRM_TTM_HELPER > + select I2C > + select I2C_ALGOBIT > select POWER_SUPPLY > select HWMON > select BACKLIGHT_CLASS_DEVICE > @@ -254,6 +255,8 @@ config DRM_AMDGPU > select DRM_SCHED > select DRM_TTM > select DRM_TTM_HELPER > + select I2C > + select I2C_ALGOBIT > select POWER_SUPPLY > select HWMON > select BACKLIGHT_CLASS_DEVICE > diff --git a/drivers/gpu/drm/ast/Kconfig b/drivers/gpu/drm/ast/Kconfig > index fbcf2f45cef5..bcc25decd485 100644 > --- a/drivers/gpu/drm/ast/Kconfig > +++ b/drivers/gpu/drm/ast/Kconfig > @@ -6,6 +6,8 @@ config DRM_AST > select DRM_VRAM_HELPER > select DRM_TTM > select DRM_TTM_HELPER > + select I2C > + select I2C_ALGOBIT > help >Say yes for experimental AST GPU driver. Do not enable >this driver without having a working -modesetting, > diff --git a/drivers/gpu/drm/gma500/Kconfig b/drivers/gpu/drm/gma500/Kconfig > index 0cff20265f97..e26c3a24955d 100644 > --- a/drivers/gpu/drm/gma500/Kconfig > +++ b/drivers/gpu/drm/gma500/Kconfig > @@ -3,6 +3,8 @@ config DRM_GMA500 > tristate "Intel GMA500/600/3600/3650 KMS Framebuffer" > depends on DRM && PCI && X86 && MMU > select DRM_KMS_HELPER > + select I2C > + select I2C_ALGOBIT > # GMA500 depends on ACPI_VIDEO when ACPI is enabled, just like i915 > select ACPI_VIDEO if ACPI > select BACKLIGHT_CLASS_DEVICE if ACPI > diff --git a/drivers/gpu/drm/hisilicon/hibmc/Kconfig > b/drivers/gpu/drm/hisilicon/hibmc/Kconfig > index 43943e980203..ac8c42dc79f6 100644 > --- a/drivers/gpu/drm/hisilicon/hibmc/Kconfig > +++ b/drivers/gpu/drm/hisilicon/hibmc/Kconfig > @@ -6,6 +6,8 @@ config DRM_HISI_HIBMC > select DRM_VRAM_HELPER > select DRM_TTM > select DRM_TTM_HELPER > + select I2C > + select I2C_ALGOBIT > help > Choose this option if you have a Hisilicon Hibmc soc chipset. > If M is selected the module will be called hibmc-drm. > diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig > index 69f57ca9c68d..b3bb6f7cfbbc 100644 > --- a/drivers/gpu/drm/i915/Kconfig > +++ b/drivers/gpu/drm/i915/Kconfig > @@ -13,6 +13,8 @@ config DRM_I915 > select DRM_PANEL > select DRM_MIPI_DSI > select RELAY > + select I2C > + select I2C_ALGOBIT > select IRQ_WORK > # i915 depends on ACPI_VIDEO when ACPI is enabled > # but for select to work, need to select ACPI_VIDEO's dependencies, ick > diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig > index eec59658a938..b28c5e4828f4 100644 > --- a/drivers/gpu/drm/mgag200/Kconfig > +++ b/drivers/gpu/drm/mgag200/Kconfig > @@ -4,6 +4,8 @@ config DRM_MGAG200 > depends on DRM && PCI && MMU > select DRM_GEM_SHMEM_HELPER > select DRM_KMS_HELPER > + select I2C > + select I2C_ALGOBIT > help >This is a KMS driver for Matrox G200 chips. It supports the original >MGA G200 desktop chips and the server variants. It requires 0.3.0 > diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig > index 9436310d0854..8823f0b24c73 100644 > --- a/drivers/gpu/drm/nouveau/Kconfig > +++ b/drivers/gpu/drm/nouveau/Kconfig > @@ -7,6 +7,8 @@ config DRM_NOUVEAU >
Re: [PATCH] drm/amdgpu: check vm ready by evicting
Dear Qiang Yu, Am 21.02.22 um 11:12 schrieb Qiang Yu: Thank you for your patch. Reading the commit message summary, I have no idea what “check vm ready by evicting” means. Can you please rephrase it? Workstation application ANSA/META get this error dmesg: What version, and how can this be reproduced exactly? Just by starting the application? [drm:amdgpu_gem_va_ioctl [amdgpu]] *ERROR* Couldn't update BO_VA (-16) This is caused by: 1. create a 256MB buffer in invisible VRAM 2. CPU map the buffer and access it causes vm_fault and try to move it to visible VRAM 3. force visible VRAM space and traverse all VRAM bos to check if evicting this bo is valuable 4. when checking a VM bo (in invisible VRAM), amdgpu_vm_evictable() will set amdgpu_vm->evicting, but latter due to not in visible VRAM, won't really evict it so not add it to amdgpu_vm->evicted 5. before next CS to clear the amdgpu_vm->evicting, user VM ops ioctl will pass amdgpu_vm_ready() (check amdgpu_vm->evicted) but fail in amdgpu_vm_bo_update_mapping() (check amdgpu_vm->evicting) and get this error log This error won't affect functionality as next CS will finish the waiting VM ops. But we'd better clear the error log by check the s/check/checking/ evicting flag which really stop VM ops latter. stop*s*? Can you please elaborate. Christian’s and your discussions was quite long, so adding a summary, why this approach works and what possible regressions there are going to be might be warranted. Kind regards, Paul Signed-off-by: Qiang Yu --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 37acd8911168..2cd9f1a2e5fa 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -770,11 +770,16 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm, * Check if all VM PDs/PTs are ready for updates * * Returns: - * True if eviction list is empty. + * True if VM is not evicting. */ bool amdgpu_vm_ready(struct amdgpu_vm *vm) { - return list_empty(&vm->evicted); + bool ret; + + amdgpu_vm_eviction_lock(vm); + ret = !vm->evicting; + amdgpu_vm_eviction_unlock(vm); + return ret; } /**
Re: [Intel-gfx] [PATCH 6/7] drm: Document fdinfo format specification
Hi Rob, On 25/01/2022 10:24, Tvrtko Ursulin wrote: On 21/01/2022 11:50, Tvrtko Ursulin wrote: On 20/01/2022 16:44, Rob Clark wrote: [snip] If there is a tool somewhere that displays this info, that would be useful for testing my implementation. I have a patch to Intel specific intel_gpu_top (see https://patchwork.freedesktop.org/patch/468491/?series=98555&rev=1). I'll have a look to see how much work would it be to extract common bits into a library and write a quick agnostic tool using it. I factored out some code from intel_gpu_top in a quick and dirty attempt to make it generic and made a very rudimentary tools/gputop: https://cgit.freedesktop.org/~tursulin/intel-gpu-tools/log/?h=gputop Have you managed to spend any time playing with this yet? The only remaining open was Daniel's mild concern if vendor agnostic userspace is possible using the proposed spec. If you managed to wire up the compliant exports and gputop tool works I think that concern would be settled. Regards, Tvrtko
Re: [PATCH v4 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels
Den 18.02.2022 16.11, skrev Noralf Trønnes: > Add binding for MIPI DBI compatible SPI panels. > > v4: > - There should only be two compatible (Maxime) > - s/panel-dbi-spi/panel-mipi-dbi-spi/in compatible > > v3: > - Move properties to Device Tree (Maxime) > - Use contains for compatible (Maxime) > - Add backlight property to example > - Flesh out description > > v2: > - Fix path for panel-common.yaml > - Use unevaluatedProperties > - Drop properties which are in the allOf section > - Drop model property (Rob) > > Acked-by: Maxime Ripard > Signed-off-by: Noralf Trønnes > --- > .../display/panel/panel-mipi-dbi-spi.yaml | 125 ++ > 1 file changed, 125 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml > > diff --git > a/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml > b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml > new file mode 100644 > index ..748c09113168 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml > +allOf: > + - $ref: panel-common.yaml# > + - $ref: /schemas/spi/spi-peripheral-props.yaml# > + > +properties: > + compatible: > +items: > + - {} # Panel Specific Compatible > + - const: panel-mipi-dbi-spi > + Rob's bot uses a -m flag I didn't use, and with that the compatible fails: $ make DT_CHECKER_FLAGS=-m dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml CHKDT Documentation/devicetree/bindings/processed-schema-examples.json SCHEMA Documentation/devicetree/bindings/processed-schema-examples.json DTEX Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dts DTC Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml CHECK Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml:0:0: /example-0/spi/display@0: failed to match any schema with compatible: ['sainsmart18', 'panel-mipi-dbi-spi'] How can I write the compatible property to make the checker happy? Noralf. > + write-only: > +type: boolean > +description: > + Controller is not readable (ie. MISO is not wired up). > + > + dc-gpios: > +maxItems: 1 > +description: | > + Controller data/command selection (D/CX) in 4-line SPI mode. > + If not set, the controller is in 3-line SPI mode. > + > +required: > + - compatible > + - reg > + - panel-timing > + > +unevaluatedProperties: false > + > +examples: > + - | > +#include > + > +spi { > +#address-cells = <1>; > +#size-cells = <0>; > + > +display@0{ > +compatible = "sainsmart18", "panel-mipi-dbi-spi"; > +reg = <0>; > +spi-max-frequency = <4000>; > + > +dc-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>; > +reset-gpios = <&gpio 25 GPIO_ACTIVE_HIGH>; > +write-only; > + > +backlight = <&backlight>; > + > +width-mm = <35>; > +height-mm = <28>; > + > +panel-timing { > +hactive = <160>; > +vactive = <128>; > +hback-porch = <0>; > +vback-porch = <0>; > + > +clock-frequency = <0>; > +hfront-porch = <0>; > +hsync-len = <0>; > +vfront-porch = <0>; > +vsync-len = <0>; > +}; > +}; > +}; > + > +...
[PATCH] drm/amd/display: move FPU-related code from dcn20 to dml folder
Move parts of dcn20 code that uses FPU to dml folder. It aims to isolate FPU operations as described by series: drm/amd/display: Introduce FPU directory inside DC https://patchwork.freedesktop.org/series/93042/ This patch moves the following functions from dcn20_resource to dml/dcn20_fpu and calls of public functions in dcn20_resource are wrapped by DC_FP_START/END(): - void dcn20_populate_dml_writeback_from_context - static bool is_dtbclk_required() - static enum dcn_zstate_support_state() - void dcn20_calculate_dlg_params() - static void swizzle_to_dml_params() - int dcn20_populate_dml_pipes_from_context() - void dcn20_calculate_wm() - void dcn20_cap_soc_clocks() - void dcn20_update_bounding_box() - void dcn20_patch_bounding_box() - bool dcn20_validate_bandwidth_fp() This movement also affects dcn30/31, as dcn20_calculate_dlg_params() is used by dcn30 and dcn31. For this reason, I included dcn20_fpu headers in dcn20_resource headers to make dcn20_calculate_dlg_params() visible to dcn30/31. Three new functions are created to isolate well-delimited FPU operations: - void dcn20_fpu_set_wb_arb_params(): set cli_watermark, pstate_watermark and time_per_pixel from wb_arb_params (struct mcif_arb_params), since those uses FPU operations on double types: WritebackUrgentWatermark, WritebackDRAMClockChangeWatermark, '16.0'. - void dcn20_fpu_set_wm_ranges(): set min_fill_clk_mhz and max_fill_clk_mhz involves FPU calcs on dram_speed_mts (double type); - void dcn20_fpu_adjust_dppclk(): adjust operation on RequiredDPPCLK that is a double. Signed-off-by: Melissa Wen --- drivers/gpu/drm/amd/display/dc/dcn20/Makefile | 25 - .../drm/amd/display/dc/dcn20/dcn20_resource.c | 1370 +--- .../drm/amd/display/dc/dcn20/dcn20_resource.h | 30 +- .../drm/amd/display/dc/dml/dcn20/dcn20_fpu.c | 1385 + .../drm/amd/display/dc/dml/dcn20/dcn20_fpu.h | 42 +- 5 files changed, 1451 insertions(+), 1401 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/Makefile b/drivers/gpu/drm/amd/display/dc/dcn20/Makefile index 5fcaf78334ff..abaed2121feb 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn20/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dcn20/Makefile @@ -9,31 +9,6 @@ DCN20 = dcn20_resource.o dcn20_init.o dcn20_hwseq.o dcn20_dpp.o dcn20_dpp_cm.o d DCN20 += dcn20_dsc.o -ifdef CONFIG_X86 -CFLAGS_$(AMDDALPATH)/dc/dcn20/dcn20_resource.o := -mhard-float -msse -endif - -ifdef CONFIG_PPC64 -CFLAGS_$(AMDDALPATH)/dc/dcn20/dcn20_resource.o := -mhard-float -maltivec -endif - -ifdef CONFIG_CC_IS_GCC -ifeq ($(call cc-ifversion, -lt, 0701, y), y) -IS_OLD_GCC = 1 -endif -endif - -ifdef CONFIG_X86 -ifdef IS_OLD_GCC -# Stack alignment mismatch, proceed with caution. -# GCC < 7.1 cannot compile code using `double` and -mpreferred-stack-boundary=3 -# (8B stack alignment). -CFLAGS_$(AMDDALPATH)/dc/dcn20/dcn20_resource.o += -mpreferred-stack-boundary=4 -else -CFLAGS_$(AMDDALPATH)/dc/dcn20/dcn20_resource.o += -msse2 -endif -endif - AMD_DAL_DCN20 = $(addprefix $(AMDDALPATH)/dc/dcn20/,$(DCN20)) AMD_DISPLAY_FILES += $(AMD_DAL_DCN20) diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c index dfe2e1c25a26..63c50bee0144 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c @@ -63,7 +63,6 @@ #include "dcn20_dccg.h" #include "dcn20_vmid.h" #include "dc_link_ddc.h" -#include "dc_link_dp.h" #include "dce/dce_panel_cntl.h" #include "navi10_ip_offset.h" @@ -93,367 +92,6 @@ #define DC_LOGGER_INIT(logger) -struct _vcs_dpi_ip_params_st dcn2_0_ip = { - .odm_capable = 1, - .gpuvm_enable = 0, - .hostvm_enable = 0, - .gpuvm_max_page_table_levels = 4, - .hostvm_max_page_table_levels = 4, - .hostvm_cached_page_table_levels = 0, - .pte_group_size_bytes = 2048, - .num_dsc = 6, - .rob_buffer_size_kbytes = 168, - .det_buffer_size_kbytes = 164, - .dpte_buffer_size_in_pte_reqs_luma = 84, - .pde_proc_buffer_size_64k_reqs = 48, - .dpp_output_buffer_pixels = 2560, - .opp_output_buffer_lines = 1, - .pixel_chunk_size_kbytes = 8, - .pte_chunk_size_kbytes = 2, - .meta_chunk_size_kbytes = 2, - .writeback_chunk_size_kbytes = 2, - .line_buffer_size_bits = 789504, - .is_line_buffer_bpp_fixed = 0, - .line_buffer_fixed_bpp = 0, - .dcc_supported = true, - .max_line_buffer_lines = 12, - .writeback_luma_buffer_size_kbytes = 12, - .writeback_chroma_buffer_size_kbytes = 8, - .writeback_chroma_line_buffer_width_pixels = 4, - .writeback_max_hscl_ratio = 1, - .writeback_max_vscl_ratio = 1, - .writeback_min_hscl_ratio = 1, - .writeback_min_vscl_ratio = 1, - .writeback_max_hscl_taps = 12, - .writeback_max_vscl_taps = 12, - .writeback_line_buffer_luma_buffer_size = 0, - .writeback_line_
Re: [PATCH v3 3/9] gpu: host1x: Add context device management code
On 2/19/22 19:48, Dmitry Osipenko wrote: 18.02.2022 14:39, Mikko Perttunen пишет: ... +/* + * Due to an issue with T194 NVENC, only 38 bits can be used. + * Anyway, 256GiB of IOVA ought to be enough for anyone. + */ +static dma_addr_t context_device_dma_mask = DMA_BIT_MASK(38); s/dma_addr_t/u64/ ? Apparently you should get compilation warning on ARM32. https://elixir.bootlin.com/linux/v5.17-rc4/source/include/linux/device.h#L524 > +int host1x_context_list_init(struct host1x *host1x) +{ + struct host1x_context_list *cdl = &host1x->context_list; + struct host1x_context *ctx; + struct device_node *node; + int index; Nitpick: unsigned int ... +del_devices: + while (--index >= 0) Nitpick: while (index--) ... >> +void host1x_context_list_free(struct host1x_context_list *cdl) +{ + int i; Nitpick: unsigned int Thanks, fixed all. Mikko
Re: [PATCH v3 3/9] gpu: host1x: Add context device management code
On 2/19/22 19:52, Dmitry Osipenko wrote: 18.02.2022 14:39, Mikko Perttunen пишет: + for (index = 0; index < cdl->len; index++) { + struct iommu_fwspec *fwspec; + + ctx = &cdl->devs[index]; + + ctx->host = host1x; + + device_initialize(&ctx->dev); + + ctx->dev.dma_mask = &context_device_dma_mask; + ctx->dev.coherent_dma_mask = context_device_dma_mask; + dev_set_name(&ctx->dev, "host1x-ctx.%d", index); + ctx->dev.bus = &host1x_context_device_bus_type; host1x_context_device_bus_type will be an undefined symbol if CONFIG_TEGRA_HOST1X_CONTEXT_BUS=n? Please compile and test all combinations. But this file is only built if CONFIG_HOST1X, which selects CONFIG_TEGRA_HOST1X_CONTEXT_BUS? Mikko
Re: [PATCH v3 8/9] drm/tegra: vic: Implement get_streamid_offset
On 2/19/22 20:54, Dmitry Osipenko wrote: 19.02.2022 21:49, Dmitry Osipenko пишет: 18.02.2022 14:39, Mikko Perttunen пишет: +static int vic_get_streamid_offset(struct tegra_drm_client *client) +{ + struct vic *vic = to_vic(client); + int err; + + err = vic_load_firmware(vic); You can't invoke vic_load_firmware() while RPM is suspended. Either replace this with RPM get/put or do something else. Why not, I'm not seeing any HW accesses in vic_load_firmware? Although it looks like it might race with the vic_load_firmware call in vic_runtime_resume which probably needs to be fixed. + if (err < 0) + return err; + + if (vic->can_use_context) + return 0x30; + else + return -ENOTSUPP; If (!vic->can_use_context) return -ENOTSUPP; return 0x30; and s/ENOTSUPP/EOPNOTSUPP/ Ok. Mikko
Re: [PATCH v6 21/23] drm: rockchip: Add VOP2 driver
Hi Sascha: On 2/17/22 16:29, Sascha Hauer wrote: From: Andy Yan The VOP2 unit is found on Rockchip SoCs beginning with rk3566/rk3568. It replaces the VOP unit found in the older Rockchip SoCs. This driver has been derived from the downstream Rockchip Kernel and heavily modified: - All nonstandard DRM properties have been removed - dropped struct vop2_plane_state and pass around less data between functions - Dropped all DRM_FORMAT_* not known on upstream - rework register access to get rid of excessively used macros - Drop all waiting for framesyncs All the waiting sync in the downstream divers are try to fix special problems, and some of them inherited from upstream vop driver, for example: the fb_unref_work, It submitted by Tomas Figa to upstream vop driver to make the wait for flip and asynchronously cursor work right. VOP2 share the same hardware design in vblank, so I don't think these code are useless. [0] https://patchwork.kernel.org/project/linux-rockchip/patch/1473857701-9250-5-git-send-email-tf...@chromium.org/ [1] https://patchwork.kernel.org/project/linux-rockchip/patch/1473857701-9250-6-git-send-email-tf...@chromium.org/ [2] https://patchwork.kernel.org/project/linux-rockchip/patch/1473857701-9250-4-git-send-email-tf...@chromium.org/ The driver is tested with HDMI and MIPI-DSI display on a RK3568-EVB board. Overlay support is tested with the modetest utility. AFBC support on the cluster windows is tested with weston-simple-dmabuf-egl on weston using the (yet to be upstreamed) panfrost driver support. Signed-off-by: Andy Yan Signed-off-by: Sascha Hauer --- Notes: Changes since v5: - consistently use u8/u16/u32 rather than uint8_t/uint16_t/uint32_t - Use spin_lock rather than spin_lock_irqsave - replace printk with drm_dbg - break some overlong lines Changes since v4: - Avoid stack frame overflow by not allocating big array on the stack Changes since v3: - Sort includes - fix typos - Drop spinlock - Use regmap_set_bits()/regmap_clear_bits() - simplify vop2_scale_factor() - simplify vop2_afbc_transform_offset() Changes since v4: - Sort nodes alphabetically Changes since v3: - Fix HDMI connector type drivers/gpu/drm/rockchip/Kconfig |6 + drivers/gpu/drm/rockchip/Makefile|1 + drivers/gpu/drm/rockchip/rockchip_drm_drv.c |1 + drivers/gpu/drm/rockchip/rockchip_drm_drv.h |6 +- drivers/gpu/drm/rockchip/rockchip_drm_fb.c |2 + drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 15 + drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 2708 ++ drivers/gpu/drm/rockchip/rockchip_drm_vop2.h | 477 +++ drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 281 ++ 9 files changed, 3496 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_vop2.h create mode 100644 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig index b9b156308460a..4ff0043f0ee70 100644 --- a/drivers/gpu/drm/rockchip/Kconfig +++ b/drivers/gpu/drm/rockchip/Kconfig @@ -28,6 +28,12 @@ config ROCKCHIP_VOP This selects support for the VOP driver. You should enable it on all older SoCs up to RK3399. +config ROCKCHIP_VOP2 + bool "Rockchip VOP2 driver" + help + This selects support for the VOP2 driver. You should enable it + on all newer SoCs beginning form RK3568. + config ROCKCHIP_ANALOGIX_DP bool "Rockchip specific extensions for Analogix DP driver" depends on ROCKCHIP_VOP diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile index dfc5512fdb9f1..3ff7b21c04149 100644 --- a/drivers/gpu/drm/rockchip/Makefile +++ b/drivers/gpu/drm/rockchip/Makefile @@ -6,6 +6,7 @@ rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \ rockchip_drm_gem.o +rockchipdrm-$(CONFIG_ROCKCHIP_VOP2) += rockchip_drm_vop2.o rockchip_vop2_reg.o rockchipdrm-$(CONFIG_ROCKCHIP_VOP) += rockchip_drm_vop.o rockchip_vop_reg.o rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c index 82c8faf1fb6b8..95f6c5985fdd7 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@ -459,6 +459,7 @@ static int __init rockchip_drm_init(void) num_rockchip_sub_drivers = 0; ADD_ROCKCHIP_SUB_DRIVER(vop_platform_driver, CONFIG_ROCKCHIP_VOP); + ADD_ROCKCHIP_SUB_DRIVER(vop2_platform_driver, CONFIG_ROCKCHIP_VOP2); ADD_ROCKCHIP_SUB_DRIVER(rockchip_lvds_driver, CONFIG_ROCKCHIP_LVDS);
Re: [PATCH V2] drm/imx: parallel-display: Remove bus flags check in imx_pd_bridge_atomic_check()
Am Montag, den 21.02.2022, 09:29 +0100 schrieb Boris Brezillon: > Hello Christoph, > > On Sat, 19 Feb 2022 09:28:44 + > Christoph Niedermaier wrote: > > > From: Max Krummenacher [mailto:max.oss...@gmail.com] > > Sent: Wednesday, February 9, 2022 10:38 AM > > > > If display timings were read from the devicetree using > > > > of_get_display_timing() and pixelclk-active is defined > > > > there, the flag DISPLAY_FLAGS_SYNC_POSEDGE/NEGEDGE is > > > > automatically generated. Through the function > > > > drm_bus_flags_from_videomode() e.g. called in the > > > > panel-simple driver this flag got into the bus flags, > > > > but then in imx_pd_bridge_atomic_check() the bus flag > > > > check failed and will not initialize the display. The > > > > original commit fe141cedc433 does not explain why this > > > > check was introduced. So remove the bus flags check, > > > > because it stops the initialization of the display with > > > > valid bus flags. > > > > > > > > Fixes: fe141cedc433 ("drm/imx: pd: Use bus format/flags provided by the > > > > bridge when > > > > available") > > > > Signed-off-by: Christoph Niedermaier > > > > Cc: Marek Vasut > > > > Cc: Boris Brezillon > > > > Cc: Philipp Zabel > > > > Cc: David Airlie > > > > Cc: Daniel Vetter > > > > Cc: Shawn Guo > > > > Cc: Sascha Hauer > > > > Cc: Pengutronix Kernel Team > > > > Cc: Fabio Estevam > > > > Cc: NXP Linux Team > > > > Cc: linux-arm-ker...@lists.infradead.org > > > > To: dri-devel@lists.freedesktop.org > > > > --- > > > > V2: - Add Boris to the Cc list > > > > --- > > > > drivers/gpu/drm/imx/parallel-display.c | 8 > > > > 1 file changed, 8 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/imx/parallel-display.c > > > > b/drivers/gpu/drm/imx/parallel-display.c > > > > index a8aba0141ce7..06cb1a59b9bc 100644 > > > > --- a/drivers/gpu/drm/imx/parallel-display.c > > > > +++ b/drivers/gpu/drm/imx/parallel-display.c > > > > @@ -217,14 +217,6 @@ static int imx_pd_bridge_atomic_check(struct > > > > drm_bridge *bridge, > > > > if (!imx_pd_format_supported(bus_fmt)) > > > > return -EINVAL; > > > > > > > > - if (bus_flags & > > > > - ~(DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_DE_HIGH | > > > > - DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE | > > > > - DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)) { > > > > - dev_warn(imxpd->dev, "invalid bus_flags (%x)\n", > > > > bus_flags); > > > > - return -EINVAL; > > > > - } > > > > - > > > > bridge_state->output_bus_cfg.flags = bus_flags; > > > > bridge_state->input_bus_cfg.flags = bus_flags; > > > > imx_crtc_state->bus_flags = bus_flags; > > > > > > Tested on a Colibri iMX6DL with a panel-dpi based panel. > > > > > > Tested-by: Max Krummenacher > > > > I still ask myself why this bus flag check is in the code. > > Is there a reason not to remove that? > > The reasoning was that DE_{LOW,HIGH} and > FLAG_PIXDATA_DRIVE_{POS,NEG}EDGE were the only bus_flags taken into > account by the crtc logic, so anything else is simply ignored. This was > definitely wrong since the driver supports at least one of the VSYNC > polarity (perhaps both if there's a way to configure it that's not > hooked-up yet). > > So I guess figuring out the default VSYNC polarity and accepting the > according DISPLAY_FLAGS_SYNC_XXXEDGE flag is what makes most sense here. Note that {HV}SYNC polarities are taken from the timings '.flag' field The bus_flags do not carry that information. (drivers/gpu/ipu-v3/ipu-di.c:611:if (sig->mode.flags & DISPLAY_FLAGS_HSYNC_HIGH)) The new flags DRM_BUS_FLAG_SYNC_DRIVE_{POS,NEG}EDGE are siblings to DRM_BUS_FLAG_PIXDATA_DRIVE_{POS,NEG}EDGE and would allow to specify on which pixelclock edge the sync signals are to be driven. Before that addition it was implicitly assumed that the sync signals and data signals would be driven on the same clock edge. The way I read the IPU driver it is not foreseen that the data lines and sync lines are driven by a different clock edge. I personally would just drop the sanity test on bus_flags. This is what this patch proposes. Regards, Max > > Regards, > > Boris
Re: [PATCH v3 9/9] drm/tegra: Support context isolation
On 2/19/22 20:35, Dmitry Osipenko wrote: 18.02.2022 14:39, Mikko Perttunen пишет: + if (context->memory_context && context->client->ops->get_streamid_offset) { ^^^ + int offset = context->client->ops->get_streamid_offset(context->client); + + if (offset >= 0) { + job->context = context->memory_context; + job->engine_streamid_offset = offset; + host1x_context_get(job->context); + } You should bump refcount unconditionally or you'll get refcnt underflow on put, when offset < 0. This refcount is intended to be dropped from 'release_job', where it's dropped if job->context is set, which it is from this path. + } + /* * job_data is now part of job reference counting, so don't release * it from here. diff --git a/drivers/gpu/drm/tegra/uapi.c b/drivers/gpu/drm/tegra/uapi.c index 9ab9179d2026..be33da54d12c 100644 --- a/drivers/gpu/drm/tegra/uapi.c +++ b/drivers/gpu/drm/tegra/uapi.c @@ -33,6 +33,9 @@ static void tegra_drm_channel_context_close(struct tegra_drm_context *context) struct tegra_drm_mapping *mapping; unsigned long id; + if (context->memory_context) + host1x_context_put(context->memory_context); The "if (context->memory_context && context->client->ops->get_streamid_offset)" above doesn't match the "if (context->memory_context)". You'll get refcount underflow. And this drop is for the refcount implicitly added when allocating the memory context through host1x_context_alloc; so these two places should be independent. Please elaborate if I missed something. Thanks, Mikko
[PATCH v2] drm/i915/ttm: fixup the mock_bo
When running the mock selftests we currently blow up with: <6> [299.836278] i915: Running i915_gem_huge_page_mock_selftests/igt_mock_memory_region_huge_pages <1> [299.836356] BUG: kernel NULL pointer dereference, address: 00c8 <1> [299.836361] #PF: supervisor read access in kernel mode <1> [299.836364] #PF: error_code(0x) - not-present page <6> [299.836367] PGD 0 P4D 0 <4> [299.836369] Oops: [#1] PREEMPT SMP NOPTI <4> [299.836372] CPU: 1 PID: 1429 Comm: i915_selftest Tainted: G U 5.17.0-rc4-CI-CI_DRM_11227+ #1 <4> [299.836376] Hardware name: Intel(R) Client Systems NUC11TNHi5/NUC11TNBi5, BIOS TNTGL357.0042.2020.1221.1743 12/21/2020 <4> [299.836380] RIP: 0010:ttm_resource_init+0x57/0x90 [ttm] <4> [299.836392] RSP: 0018:c90001e4f680 EFLAGS: 00010203 <4> [299.836395] RAX: RBX: c90001e4f708 RCX: <4> [299.836398] RDX: 888116172528 RSI: c90001e4f6f8 RDI: <4> [299.836401] RBP: c90001e4f6f8 R08: 01b0 R09: 888116172528 <4> [299.836403] R10: 0001 R11: a4cb2e51 R12: c90001e4fa90 <4> [299.836406] R13: 888116172528 R14: 888130d7f4b0 R15: 888130d7f400 <4> [299.836409] FS: 7ff241684500() GS:88849fe8() knlGS: <4> [299.836412] CS: 0010 DS: ES: CR0: 80050033 <4> [299.836416] CR2: 00c8 CR3: 000107b80001 CR4: 00770ee0 <4> [299.836418] PKRU: 5554 <4> [299.836420] Call Trace: <4> [299.836422] <4> [299.836423] i915_ttm_buddy_man_alloc+0x68/0x240 [i915] ttm_resource_init() now needs to access the bo->bdev, and also wants to store the bo reference. Try to keep both working. The mock_bo is a hack so we can interface directly with the ttm managers alloc() and free() hooks for our mock testing, without invoking other TTM features like eviction, moves, etc. v2: make sure we only touch res->bo if the alloc() returns successfully Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5123 Fixes: 0e05fc49c358 ("drm/ttm: add common accounting to the resource mgr v3") Signed-off-by: Matthew Auld Cc: Christian König Cc: Thomas Hellström Acked-by: Christian König --- drivers/gpu/drm/i915/intel_region_ttm.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c b/drivers/gpu/drm/i915/intel_region_ttm.c index f2b888c16958..7dea07c579aa 100644 --- a/drivers/gpu/drm/i915/intel_region_ttm.c +++ b/drivers/gpu/drm/i915/intel_region_ttm.c @@ -200,11 +200,14 @@ intel_region_ttm_resource_alloc(struct intel_memory_region *mem, int ret; mock_bo.base.size = size; + mock_bo.bdev = &mem->i915->bdev; place.flags = flags; ret = man->func->alloc(man, &mock_bo, &place, &res); if (ret == -ENOSPC) ret = -ENXIO; + if (!ret) + res->bo = NULL; /* Rather blow up, then some uaf */ return ret ? ERR_PTR(ret) : res; } @@ -219,6 +222,11 @@ void intel_region_ttm_resource_free(struct intel_memory_region *mem, struct ttm_resource *res) { struct ttm_resource_manager *man = mem->region_private; + struct ttm_buffer_object mock_bo = {}; + + mock_bo.base.size = res->num_pages << PAGE_SHIFT; + mock_bo.bdev = &mem->i915->bdev; + res->bo = &mock_bo; man->func->free(man, res); } -- 2.34.1
Re: [PATCH] drm: rcar-du: switch to devm_drm_of_get_bridge
On Mon, Feb 21, 2022 at 09:56:19AM +0100, Maxime Ripard wrote: > On Mon, Feb 21, 2022 at 08:37:57AM +0100, José Expósito wrote: > > The function "drm_of_find_panel_or_bridge" has been deprecated in > > favor of "devm_drm_of_get_bridge". > > > > Switch to the new function and reduce boilerplate. > > > > Signed-off-by: José Expósito > > --- > > drivers/gpu/drm/rcar-du/rcar_lvds.c | 16 +--- > > 1 file changed, 5 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c > > b/drivers/gpu/drm/rcar-du/rcar_lvds.c > > index 72a272cfc11e..99b0febc56d1 100644 > > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c > > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c > > @@ -712,18 +712,12 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) > > { > > int ret; > > > > - ret = drm_of_find_panel_or_bridge(lvds->dev->of_node, 1, 0, > > - &lvds->panel, &lvds->next_bridge); > > I guess lvds->panel can be removed from the rcar_lvds struct as well? It's used in rcar_lvds_get_lvds_mode() though, so this patch introduces a regression. -- Regards, Laurent Pinchart
Re: [PATCH V2] drm/imx: parallel-display: Remove bus flags check in imx_pd_bridge_atomic_check()
On Mon, 21 Feb 2022 12:56:43 +0100 Max Krummenacher wrote: > Am Montag, den 21.02.2022, 09:29 +0100 schrieb Boris Brezillon: > > Hello Christoph, > > > > On Sat, 19 Feb 2022 09:28:44 + > > Christoph Niedermaier wrote: > > > > > From: Max Krummenacher [mailto:max.oss...@gmail.com] > > > Sent: Wednesday, February 9, 2022 10:38 AM > > > > > If display timings were read from the devicetree using > > > > > of_get_display_timing() and pixelclk-active is defined > > > > > there, the flag DISPLAY_FLAGS_SYNC_POSEDGE/NEGEDGE is > > > > > automatically generated. Through the function > > > > > drm_bus_flags_from_videomode() e.g. called in the > > > > > panel-simple driver this flag got into the bus flags, > > > > > but then in imx_pd_bridge_atomic_check() the bus flag > > > > > check failed and will not initialize the display. The > > > > > original commit fe141cedc433 does not explain why this > > > > > check was introduced. So remove the bus flags check, > > > > > because it stops the initialization of the display with > > > > > valid bus flags. > > > > > > > > > > Fixes: fe141cedc433 ("drm/imx: pd: Use bus format/flags provided by > > > > > the bridge when > > > > > available") > > > > > Signed-off-by: Christoph Niedermaier > > > > > Cc: Marek Vasut > > > > > Cc: Boris Brezillon > > > > > Cc: Philipp Zabel > > > > > Cc: David Airlie > > > > > Cc: Daniel Vetter > > > > > Cc: Shawn Guo > > > > > Cc: Sascha Hauer > > > > > Cc: Pengutronix Kernel Team > > > > > Cc: Fabio Estevam > > > > > Cc: NXP Linux Team > > > > > Cc: linux-arm-ker...@lists.infradead.org > > > > > To: dri-devel@lists.freedesktop.org > > > > > --- > > > > > V2: - Add Boris to the Cc list > > > > > --- > > > > > drivers/gpu/drm/imx/parallel-display.c | 8 > > > > > 1 file changed, 8 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/imx/parallel-display.c > > > > > b/drivers/gpu/drm/imx/parallel-display.c > > > > > index a8aba0141ce7..06cb1a59b9bc 100644 > > > > > --- a/drivers/gpu/drm/imx/parallel-display.c > > > > > +++ b/drivers/gpu/drm/imx/parallel-display.c > > > > > @@ -217,14 +217,6 @@ static int imx_pd_bridge_atomic_check(struct > > > > > drm_bridge *bridge, > > > > > if (!imx_pd_format_supported(bus_fmt)) > > > > > return -EINVAL; > > > > > > > > > > - if (bus_flags & > > > > > - ~(DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_DE_HIGH | > > > > > - DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE | > > > > > - DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)) { > > > > > - dev_warn(imxpd->dev, "invalid bus_flags (%x)\n", > > > > > bus_flags); > > > > > - return -EINVAL; > > > > > - } > > > > > - > > > > > bridge_state->output_bus_cfg.flags = bus_flags; > > > > > bridge_state->input_bus_cfg.flags = bus_flags; > > > > > imx_crtc_state->bus_flags = bus_flags; > > > > > > > > Tested on a Colibri iMX6DL with a panel-dpi based panel. > > > > > > > > Tested-by: Max Krummenacher > > > > > > I still ask myself why this bus flag check is in the code. > > > Is there a reason not to remove that? > > > > The reasoning was that DE_{LOW,HIGH} and > > FLAG_PIXDATA_DRIVE_{POS,NEG}EDGE were the only bus_flags taken into > > account by the crtc logic, so anything else is simply ignored. This was > > definitely wrong since the driver supports at least one of the VSYNC > > polarity (perhaps both if there's a way to configure it that's not > > hooked-up yet). > > > > So I guess figuring out the default VSYNC polarity and accepting the > > according DISPLAY_FLAGS_SYNC_XXXEDGE flag is what makes most sense here. > > Note that {HV}SYNC polarities are taken from the timings '.flag' field > The bus_flags do not carry > that information. > (drivers/gpu/ipu-v3/ipu-di.c:611:if (sig->mode.flags & > DISPLAY_FLAGS_HSYNC_HIGH)) > > The new flags DRM_BUS_FLAG_SYNC_DRIVE_{POS,NEG}EDGE are siblings to > DRM_BUS_FLAG_PIXDATA_DRIVE_{POS,NEG}EDGE and would allow to specify > on which pixelclock edge the sync signals are to be driven. > Before that addition it was implicitly assumed that the sync signals > and data signals would be driven on the same clock edge. Oh, ok. > The way I read the IPU driver it is not > foreseen that the data lines > and sync lines are driven by a different clock edge. > > I personally would just drop the sanity test on > bus_flags. This is what > this patch proposes. Sounds fine. Acked-by: Boris Brezillon
Re: [REPOST PATCH v4 02/13] drm/msm/dsi: Pass DSC params to drm_panel
On 10/02/2022 13:34, Vinod Koul wrote: When DSC is enabled, we need to pass the DSC parameters to panel driver as well, so add a dsc parameter in panel and set it when DSC is enabled Also, fetch and pass DSC configuration for DSI panels to DPU encoder, which will enable and configure DSC hardware blocks accordingly. Signed-off-by: Dmitry Baryshkov Signed-off-by: Vinod Koul --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 ++ drivers/gpu/drm/msm/dsi/dsi.c | 5 + drivers/gpu/drm/msm/dsi/dsi.h | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c | 22 ++ drivers/gpu/drm/msm/msm_drv.h | 8 include/drm/drm_panel.h | 7 +++ 6 files changed, 45 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 47fe11a84a77..ef6ddac22767 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -578,6 +578,8 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev, MSM_DISPLAY_CAP_CMD_MODE : MSM_DISPLAY_CAP_VID_MODE; + info.dsc = msm_dsi_get_dsc_config(priv->dsi[i]); + if (msm_dsi_is_bonded_dsi(priv->dsi[i]) && priv->dsi[other]) { rc = msm_dsi_modeset_init(priv->dsi[other], dev, encoder); if (rc) { diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c index 052548883d27..3aeac15e7421 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.c +++ b/drivers/gpu/drm/msm/dsi/dsi.c @@ -20,6 +20,11 @@ bool msm_dsi_is_cmd_mode(struct msm_dsi *msm_dsi) return !(host_flags & MIPI_DSI_MODE_VIDEO); } +struct msm_display_dsc_config *msm_dsi_get_dsc_config(struct msm_dsi *msm_dsi) +{ + return msm_dsi_host_get_dsc_config(msm_dsi->host); +} + static int dsi_get_phy(struct msm_dsi *msm_dsi) { struct platform_device *pdev = msm_dsi->pdev; diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h index c8dedc95428c..16cd9b2fce86 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.h +++ b/drivers/gpu/drm/msm/dsi/dsi.h @@ -152,6 +152,7 @@ int dsi_calc_clk_rate_v2(struct msm_dsi_host *msm_host, bool is_bonded_dsi); int dsi_calc_clk_rate_6g(struct msm_dsi_host *msm_host, bool is_bonded_dsi); void msm_dsi_host_snapshot(struct msm_disp_state *disp_state, struct mipi_dsi_host *host); void msm_dsi_host_test_pattern_en(struct mipi_dsi_host *host); +struct msm_display_dsc_config *msm_dsi_host_get_dsc_config(struct mipi_dsi_host *host); /* dsi phy */ struct msm_dsi_phy; diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 27553194f9fa..7e9913eff724 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -2059,9 +2059,24 @@ int msm_dsi_host_modeset_init(struct mipi_dsi_host *host, { struct msm_dsi_host *msm_host = to_msm_dsi_host(host); const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd; + struct drm_panel *panel; int ret; msm_host->dev = dev; + panel = msm_dsi_host_get_panel(&msm_host->base); + + if (panel && panel->dsc) { + struct msm_display_dsc_config *dsc = msm_host->dsc; + + if (!dsc) { + dsc = devm_kzalloc(&msm_host->pdev->dev, sizeof(*dsc), GFP_KERNEL); + if (!dsc) + return -ENOMEM; + dsc->drm = panel->dsc; + msm_host->dsc = dsc; + } + } + ret = cfg_hnd->ops->tx_buf_alloc(msm_host, SZ_4K); if (ret) { pr_err("%s: alloc tx gem obj failed, %d\n", __func__, ret); @@ -2626,3 +2641,10 @@ void msm_dsi_host_test_pattern_en(struct mipi_dsi_host *host) dsi_write(msm_host, REG_DSI_TEST_PATTERN_GEN_CMD_STREAM0_TRIGGER, DSI_TEST_PATTERN_GEN_CMD_STREAM0_TRIGGER_SW_TRIGGER); } + +struct msm_display_dsc_config *msm_dsi_host_get_dsc_config(struct mipi_dsi_host *host) +{ + struct msm_dsi_host *msm_host = to_msm_dsi_host(host); + + return msm_host->dsc; +} diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 384f9bad4760..e7a312edfe67 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -119,6 +119,7 @@ struct msm_display_topology { * based on num_of_h_tiles * @is_te_using_watchdog_timer: Boolean to indicate watchdog TE is * used instead of panel TE in cmd mode panels + * @dsc: DSC configuration data for DSC-enabled displays */ struct msm_display_info { int intf_type; @@ -126,6 +127,7 @@ struct msm_display_info { uint32_t num_of_h_tiles; uint32_t h_tile_instance[MAX_H_TILES_PER_DISPLAY]; bool is_te_using_watchdog_timer; + stru
Re: [PATCH v2 1/3] drm/mm: Ensure that the entry is not NULL before extracting rb_node
On 18/02/2022 03:47, Kasireddy, Vivek wrote: Hi Tvrtko, On 17/02/2022 07:50, Vivek Kasireddy wrote: While looking for next holes suitable for an allocation, although, it is highly unlikely, make sure that the DECLARE_NEXT_HOLE_ADDR macro is using a valid node before it extracts the rb_node from it. Was the need for this just a consequence of insufficient locking in the i915 patch? [Kasireddy, Vivek] Partly, yes; but I figured since we are anyway doing if (!entry || ..), it makes sense to dereference entry and extract the rb_node after this check. Unless I am blind I don't see that it makes a difference. "&entry->rb_hole_addr" is taking an address of, which works "fine" is entry is NULL. And does not get past the !entry check for the actual de-reference via RB_EMPTY_NODE. With your patch you move that after the !entry check but still have it in the RB_EMPTY_NODE macro. Again, unless I am blind, I think just drop this patch. Regards, Tvrtko Thanks, Vivek Regards, Tvrtko Cc: Tvrtko Ursulin Cc: Christian König Signed-off-by: Vivek Kasireddy --- drivers/gpu/drm/drm_mm.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c index 8257f9d4f619..499d8874e4ed 100644 --- a/drivers/gpu/drm/drm_mm.c +++ b/drivers/gpu/drm/drm_mm.c @@ -389,11 +389,12 @@ first_hole(struct drm_mm *mm, #define DECLARE_NEXT_HOLE_ADDR(name, first, last)\ static struct drm_mm_node *name(struct drm_mm_node *entry, u64 size) \ {\ - struct rb_node *parent, *node = &entry->rb_hole_addr;\ + struct rb_node *parent, *node; \ \ - if (!entry || RB_EMPTY_NODE(node)) \ + if (!entry || RB_EMPTY_NODE(&entry->rb_hole_addr)) \ return NULL;\ \ + node = &entry->rb_hole_addr; \ if (usable_hole_addr(node->first, size)) { \ node = node->first; \ while (usable_hole_addr(node->last, size)) \
Re: [PATCH 1/2] drm/mm: Add an iterator to optimally walk over holes for an allocation (v3)
On 16/02/2022 10:36, Christian König wrote: Am 15.02.22 um 23:23 schrieb Vivek Kasireddy: This iterator relies on drm_mm_first_hole() and drm_mm_next_hole() functions to identify suitable holes for an allocation of a given size by efficiently traversing the rbtree associated with the given allocator. It replaces the for loop in drm_mm_insert_node_in_range() and can also be used by drm drivers to quickly identify holes of a certain size within a given range. v2: (Tvrtko) - Prepend a double underscore for the newly exported first/next_hole - s/each_best_hole/each_suitable_hole/g - Mask out DRM_MM_INSERT_ONCE from the mode before calling first/next_hole and elsewhere. v3: (Tvrtko) - Reduce the number of hunks by retaining the "mode" variable name Cc: Christian König Reviewed-by: Tvrtko Ursulin Suggested-by: Tvrtko Ursulin Signed-off-by: Vivek Kasireddy Of hand it looks like it does what the description says without any functional change, but I don't know the internals of drm_mm so well either. Feel free to add an Acked-by: Christian König . Thanks! Can we merge this via the Intel tree as one series (one drm core plus one i915 patch)? Daniel? Regards, Tvrtko
[v1] arm64/dts/qcom/sc7280: update mdp clk to max supported value to support higher refresh rates
Panels with higher refresh rate will need mdp clk above 300Mhz. Select max frequency for mdp clock during bootup, dpu driver will scale down the clock as per usecase when first update from the framework is received. Signed-off-by: Vinod Polimera --- arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi index baf1653..7af96fc 100644 --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi @@ -2895,7 +2895,7 @@ assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>, <&dispcc DISP_CC_MDSS_VSYNC_CLK>, <&dispcc DISP_CC_MDSS_AHB_CLK>; - assigned-clock-rates = <3>, + assigned-clock-rates = <50667>, <1920>, <1920>; operating-points-v2 = <&mdp_opp_table>; -- 2.7.4
Re: [PATCH v2] drm/i915/ttm: fixup the mock_bo
On Mon, 2022-02-21 at 12:11 +, Matthew Auld wrote: > When running the mock selftests we currently blow up with: > > <6> [299.836278] i915: Running > i915_gem_huge_page_mock_selftests/igt_mock_memory_region_huge_pages > <1> [299.836356] BUG: kernel NULL pointer dereference, address: > 00c8 > <1> [299.836361] #PF: supervisor read access in kernel mode > <1> [299.836364] #PF: error_code(0x) - not-present page > <6> [299.836367] PGD 0 P4D 0 > <4> [299.836369] Oops: [#1] PREEMPT SMP NOPTI > <4> [299.836372] CPU: 1 PID: 1429 Comm: i915_selftest Tainted: G > U 5.17.0-rc4-CI-CI_DRM_11227+ #1 > <4> [299.836376] Hardware name: Intel(R) Client Systems > NUC11TNHi5/NUC11TNBi5, BIOS TNTGL357.0042.2020.1221.1743 12/21/2020 > <4> [299.836380] RIP: 0010:ttm_resource_init+0x57/0x90 [ttm] > <4> [299.836392] RSP: 0018:c90001e4f680 EFLAGS: 00010203 > <4> [299.836395] RAX: RBX: c90001e4f708 RCX: > > <4> [299.836398] RDX: 888116172528 RSI: c90001e4f6f8 RDI: > > <4> [299.836401] RBP: c90001e4f6f8 R08: 01b0 R09: > 888116172528 > <4> [299.836403] R10: 0001 R11: a4cb2e51 R12: > c90001e4fa90 > <4> [299.836406] R13: 888116172528 R14: 888130d7f4b0 R15: > 888130d7f400 > <4> [299.836409] FS: 7ff241684500() > GS:88849fe8() knlGS: > <4> [299.836412] CS: 0010 DS: ES: CR0: 80050033 > <4> [299.836416] CR2: 00c8 CR3: 000107b80001 CR4: > 00770ee0 > <4> [299.836418] PKRU: 5554 > <4> [299.836420] Call Trace: > <4> [299.836422] > <4> [299.836423] i915_ttm_buddy_man_alloc+0x68/0x240 [i915] > > ttm_resource_init() now needs to access the bo->bdev, and also wants > to > store the bo reference. Try to keep both working. The mock_bo is a > hack > so we can interface directly with the ttm managers alloc() and free() > hooks for > our mock testing, without invoking other TTM features like eviction, > moves, etc. > > v2: make sure we only touch res->bo if the alloc() returns > successfully > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5123 > Fixes: 0e05fc49c358 ("drm/ttm: add common accounting to the resource > mgr v3") > Signed-off-by: Matthew Auld > Cc: Christian König > Cc: Thomas Hellström > Acked-by: Christian König Reviewed-by: Thomas Hellström > --- > drivers/gpu/drm/i915/intel_region_ttm.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c > b/drivers/gpu/drm/i915/intel_region_ttm.c > index f2b888c16958..7dea07c579aa 100644 > --- a/drivers/gpu/drm/i915/intel_region_ttm.c > +++ b/drivers/gpu/drm/i915/intel_region_ttm.c > @@ -200,11 +200,14 @@ intel_region_ttm_resource_alloc(struct > intel_memory_region *mem, > int ret; > > mock_bo.base.size = size; > + mock_bo.bdev = &mem->i915->bdev; > place.flags = flags; > > ret = man->func->alloc(man, &mock_bo, &place, &res); > if (ret == -ENOSPC) > ret = -ENXIO; > + if (!ret) > + res->bo = NULL; /* Rather blow up, then some uaf */ > return ret ? ERR_PTR(ret) : res; > } > > @@ -219,6 +222,11 @@ void intel_region_ttm_resource_free(struct > intel_memory_region *mem, > struct ttm_resource *res) > { > struct ttm_resource_manager *man = mem->region_private; > + struct ttm_buffer_object mock_bo = {}; > + > + mock_bo.base.size = res->num_pages << PAGE_SHIFT; > + mock_bo.bdev = &mem->i915->bdev; > + res->bo = &mock_bo; > > man->func->free(man, res); > }
Re: [PATCH v16 21/40] pwm: tegra: Add runtime PM and OPP support
Hello, On Mon, Feb 21, 2022 at 12:53:58PM +0300, Dmitry Osipenko wrote: > 21.02.2022 11:17, Uwe Kleine-König пишет: > >> @@ -344,7 +387,10 @@ static const struct of_device_id tegra_pwm_of_match[] > >> = { > >> MODULE_DEVICE_TABLE(of, tegra_pwm_of_match); > >> > >> static const struct dev_pm_ops tegra_pwm_pm_ops = { > >> - SET_SYSTEM_SLEEP_PM_OPS(tegra_pwm_suspend, tegra_pwm_resume) > >> + SET_RUNTIME_PM_OPS(tegra_pwm_runtime_suspend, tegra_pwm_runtime_resume, > >> + NULL) > >> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, > >> + pm_runtime_force_resume) > >> }; > >> > >> static struct platform_driver tegra_pwm_driver = { > > I admit to not completely understand the effects of this patch, but I > > don't see a problem either. So for me this patch is OK: > > > > Acked-by: Uwe Kleine-König > > > > I spot a problem, it's not introduced by this patch however: If the > > consumer of the PWM didn't stop the hardware, the suspend should IMHO be > > prevented. > > Why? The PWM driver itself will stop the h/w on suspend. Stopping the PWM might be bad. Only the consumer can know if it's ok to stop the PWM on suspend. If so the consumer should stop the PWM in their suspend callback and the PWM should prevent suspend if it wasn't stopped. > > I wonder if the patches in this series go in in one go via an ARM or > > Tegra tree, or each patch via its respective maintainer tree. > > This series, including this patch, was already applied to 5.17 via the > tegra/soc tree. No action is needed anymore. Ah, I missed that, thanks. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
[PATCH 0/8] drm/vc4: Fix frame corruption when moving the cursor
Hi, This series fixes a long standing issue with the VC4 driver when one was moving the cursor on X11 along the edges of the monitor, if we had overscan margins enabled. The details are in the commit log of the last patch, but the main reason was that moving along the edges with the overscan margins enabled triggers a full blown commit, as opposed to an async commit. Since that commit is on the cursor plane, it's treated as a legacy cursor update, and won't wait for vblank, so it's possible to queue multiple commit between vblank. Now, the composition happens in the HVS, and the HVS has a series of descriptors stored in an internal RAM, one for each plane. Allocations in that RAM are tied to the CRTC state, and freed when that state is destroyed. That internal RAM is also used by the HVS to store some internal context while it's generating a frame. If we get multiple commits between vblanks, chances are that the RAM entries used by one of the first commit is going to be freed and reused by a later commit, which will then overwrite the content of the earlier entries, erasing the context in the process and corrupting the frame. We've tested multiple solutions, but the one that seem to work without any cons is to defer the deallocation of RAM entries to the next vblank after the CRTC state has been freed. Let me know what you think, Maxime Maxime Ripard (8): drm/vc4: kms: Take old state core clock rate into account drm/vc4: hvs: Fix frame count register readout drm/vc4: hvs: Store channel in variable drm/vc4: hvs: Remove dlist setup duplication drm/vc4: hvs: Move the dlist setup to its own function drm/vc4: hvs: Ignore atomic_flush if we're disabled drm/vc4: hvs: Use pointer to HVS in HVS_READ and HVS_WRITE macros drm/vc4: hvs: Defer dlist slots deallocation drivers/gpu/drm/vc4/vc4_crtc.c | 24 +-- drivers/gpu/drm/vc4/vc4_drv.h | 30 +++- drivers/gpu/drm/vc4/vc4_hvs.c | 309 ++--- drivers/gpu/drm/vc4/vc4_kms.c | 10 +- drivers/gpu/drm/vc4/vc4_regs.h | 13 +- 5 files changed, 299 insertions(+), 87 deletions(-) -- 2.35.1
[PATCH 1/8] drm/vc4: kms: Take old state core clock rate into account
During a commit, the core clock, which feeds the HVS, needs to run at a minimum of 500MHz. While doing that commit, we can also change the mode to one that requires a higher core clock, so we take the core clock rate associated to that new state into account for that boost. However, the old state also needs to be taken into account if it requires a core clock higher that the new one and our 500MHz limit, since it's still live in hardware at the beginning of our commit. Fixes: 16e101051f32 ("drm/vc4: Increase the core clock based on HVS load") Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_kms.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index 24de29bc1cda..992d6a240002 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -385,9 +385,10 @@ static void vc4_atomic_commit_tail(struct drm_atomic_state *state) } if (vc4->hvs->hvs5) { + unsigned long state_rate = max(old_hvs_state->core_clock_rate, + new_hvs_state->core_clock_rate); unsigned long core_rate = max_t(unsigned long, - 5, - new_hvs_state->core_clock_rate); + 5, state_rate); clk_set_min_rate(hvs->core_clk, core_rate); } -- 2.35.1
[PATCH 4/8] drm/vc4: hvs: Remove dlist setup duplication
Setting the DISPLISTx register needs to occur in every case, and we don't need to protect the register using the event_lock, so we can just move it after the if branches and simplify a bit the function. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_hvs.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c index d225eea6e640..71aa5081eaa3 100644 --- a/drivers/gpu/drm/vc4/vc4_hvs.c +++ b/drivers/gpu/drm/vc4/vc4_hvs.c @@ -402,15 +402,12 @@ static void vc4_hvs_update_dlist(struct drm_crtc *crtc) crtc->state->event = NULL; } - HVS_WRITE(SCALER_DISPLISTX(vc4_state->assigned_channel), - vc4_state->mm.start); - spin_unlock_irqrestore(&dev->event_lock, flags); - } else { - HVS_WRITE(SCALER_DISPLISTX(vc4_state->assigned_channel), - vc4_state->mm.start); } + HVS_WRITE(SCALER_DISPLISTX(vc4_state->assigned_channel), + vc4_state->mm.start); + spin_lock_irqsave(&vc4_crtc->irq_lock, flags); vc4_crtc->current_dlist = vc4_state->mm.start; spin_unlock_irqrestore(&vc4_crtc->irq_lock, flags); -- 2.35.1
[PATCH 3/8] drm/vc4: hvs: Store channel in variable
The assigned_channel field of our vc4_crtc_state structure is accessed multiple times in vc4_hvs_atomic_flush, so let's move it to a variable that can be used in all those places. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_hvs.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c index c8cae10500b9..d225eea6e640 100644 --- a/drivers/gpu/drm/vc4/vc4_hvs.c +++ b/drivers/gpu/drm/vc4/vc4_hvs.c @@ -460,6 +460,7 @@ void vc4_hvs_atomic_flush(struct drm_crtc *crtc, struct drm_device *dev = crtc->dev; struct vc4_dev *vc4 = to_vc4_dev(dev); struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc->state); + unsigned int channel = vc4_state->assigned_channel; struct drm_plane *plane; struct vc4_plane_state *vc4_plane_state; bool debug_dump_regs = false; @@ -500,8 +501,8 @@ void vc4_hvs_atomic_flush(struct drm_crtc *crtc, /* This sets a black background color fill, as is the case * with other DRM drivers. */ - HVS_WRITE(SCALER_DISPBKGNDX(vc4_state->assigned_channel), - HVS_READ(SCALER_DISPBKGNDX(vc4_state->assigned_channel)) | + HVS_WRITE(SCALER_DISPBKGNDX(channel), + HVS_READ(SCALER_DISPBKGNDX(channel)) | SCALER_DISPBKGND_FILL); /* Only update DISPLIST if the CRTC was already running and is not @@ -515,7 +516,7 @@ void vc4_hvs_atomic_flush(struct drm_crtc *crtc, vc4_hvs_update_dlist(crtc); if (crtc->state->color_mgmt_changed) { - u32 dispbkgndx = HVS_READ(SCALER_DISPBKGNDX(vc4_state->assigned_channel)); + u32 dispbkgndx = HVS_READ(SCALER_DISPBKGNDX(channel)); if (crtc->state->gamma_lut) { vc4_hvs_update_gamma_lut(crtc); @@ -527,7 +528,7 @@ void vc4_hvs_atomic_flush(struct drm_crtc *crtc, */ dispbkgndx &= ~SCALER_DISPBKGND_GAMMA; } - HVS_WRITE(SCALER_DISPBKGNDX(vc4_state->assigned_channel), dispbkgndx); + HVS_WRITE(SCALER_DISPBKGNDX(channel), dispbkgndx); } if (debug_dump_regs) { -- 2.35.1
[PATCH 2/8] drm/vc4: hvs: Fix frame count register readout
In order to get the field currently being output, the driver has been using the display FIFO frame count in the HVS, reading a 6-bit field at the offset 12 in the DISPSTATx register. While that field is indeed at that location for the FIFO 1 and 2, the one for the FIFO0 is actually in the DISPSTAT1 register, at the offset 18. Fixes: e538092cb15c ("drm/vc4: Enable precise vblank timestamping for interlaced modes.") Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_crtc.c | 2 +- drivers/gpu/drm/vc4/vc4_drv.h | 1 + drivers/gpu/drm/vc4/vc4_hvs.c | 23 +++ drivers/gpu/drm/vc4/vc4_regs.h | 12 ++-- 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index e6cc47470e03..72fadce38d32 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -123,7 +123,7 @@ static bool vc4_crtc_get_scanout_position(struct drm_crtc *crtc, *vpos /= 2; /* Use hpos to correct for field offset in interlaced mode. */ - if (VC4_GET_FIELD(val, SCALER_DISPSTATX_FRAME_COUNT) % 2) + if (vc4_hvs_get_fifo_frame_count(dev, vc4_crtc_state->assigned_channel) % 2) *hpos += mode->crtc_htotal / 2; } diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 4329e09d357c..801da3e8ebdb 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -935,6 +935,7 @@ void vc4_irq_reset(struct drm_device *dev); extern struct platform_driver vc4_hvs_driver; void vc4_hvs_stop_channel(struct drm_device *dev, unsigned int output); int vc4_hvs_get_fifo_from_output(struct drm_device *dev, unsigned int output); +u8 vc4_hvs_get_fifo_frame_count(struct drm_device *dev, unsigned int fifo); int vc4_hvs_atomic_check(struct drm_crtc *crtc, struct drm_atomic_state *state); void vc4_hvs_atomic_begin(struct drm_crtc *crtc, struct drm_atomic_state *state); void vc4_hvs_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_state *state); diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c index 604933e20e6a..c8cae10500b9 100644 --- a/drivers/gpu/drm/vc4/vc4_hvs.c +++ b/drivers/gpu/drm/vc4/vc4_hvs.c @@ -197,6 +197,29 @@ static void vc4_hvs_update_gamma_lut(struct drm_crtc *crtc) vc4_hvs_lut_load(crtc); } +u8 vc4_hvs_get_fifo_frame_count(struct drm_device *dev, unsigned int fifo) +{ + struct vc4_dev *vc4 = to_vc4_dev(dev); + u8 field = 0; + + switch (fifo) { + case 0: + field = VC4_GET_FIELD(HVS_READ(SCALER_DISPSTAT1), + SCALER_DISPSTAT1_FRCNT0); + break; + case 1: + field = VC4_GET_FIELD(HVS_READ(SCALER_DISPSTAT1), + SCALER_DISPSTAT1_FRCNT1); + break; + case 2: + field = VC4_GET_FIELD(HVS_READ(SCALER_DISPSTAT2), + SCALER_DISPSTAT2_FRCNT2); + break; + } + + return field; +} + int vc4_hvs_get_fifo_from_output(struct drm_device *dev, unsigned int output) { struct vc4_dev *vc4 = to_vc4_dev(dev); diff --git a/drivers/gpu/drm/vc4/vc4_regs.h b/drivers/gpu/drm/vc4/vc4_regs.h index 33410718089e..bae8c9cd6f7c 100644 --- a/drivers/gpu/drm/vc4/vc4_regs.h +++ b/drivers/gpu/drm/vc4/vc4_regs.h @@ -379,8 +379,6 @@ # define SCALER_DISPSTATX_MODE_EOF 3 # define SCALER_DISPSTATX_FULL BIT(29) # define SCALER_DISPSTATX_EMPTYBIT(28) -# define SCALER_DISPSTATX_FRAME_COUNT_MASK VC4_MASK(17, 12) -# define SCALER_DISPSTATX_FRAME_COUNT_SHIFT12 # define SCALER_DISPSTATX_LINE_MASKVC4_MASK(11, 0) # define SCALER_DISPSTATX_LINE_SHIFT 0 @@ -403,9 +401,15 @@ (x) * (SCALER_DISPBKGND1 - \ SCALER_DISPBKGND0)) #define SCALER_DISPSTAT10x0058 +# define SCALER_DISPSTAT1_FRCNT0_MASK VC4_MASK(23, 18) +# define SCALER_DISPSTAT1_FRCNT0_SHIFT 18 +# define SCALER_DISPSTAT1_FRCNT1_MASK VC4_MASK(17, 12) +# define SCALER_DISPSTAT1_FRCNT1_SHIFT 12 + #define SCALER_DISPSTATX(x)(SCALER_DISPSTAT0 +\ (x) * (SCALER_DISPSTAT1 - \ SCALER_DISPSTAT0)) + #define SCALER_DISPBASE10x005c #define SCALER_DISPBASEX(x)(SCALER_DISPBASE0 +\ (x) * (SCALER_DISPBASE1 - \ @@ -415,7 +419,11 @@ (x) * (SCALER_DISPCTRL1 - \ SCALER_DISPCTRL0)) #define SCALER_DISPBKGND2
[PATCH 5/8] drm/vc4: hvs: Move the dlist setup to its own function
The vc4_hvs_update_dlist function mostly deals with setting up the vblank events and setting up the dlist entry pointer to our current active one. We'll want to do the former separately from the vblank handling in later patches, so let's move it to a function of its own. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_hvs.c | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c index 71aa5081eaa3..2d540fc11357 100644 --- a/drivers/gpu/drm/vc4/vc4_hvs.c +++ b/drivers/gpu/drm/vc4/vc4_hvs.c @@ -382,10 +382,19 @@ int vc4_hvs_atomic_check(struct drm_crtc *crtc, struct drm_atomic_state *state) return 0; } +static void vc4_hvs_install_dlist(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct vc4_dev *vc4 = to_vc4_dev(dev); + struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc->state); + + HVS_WRITE(SCALER_DISPLISTX(vc4_state->assigned_channel), + vc4_state->mm.start); +} + static void vc4_hvs_update_dlist(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; - struct vc4_dev *vc4 = to_vc4_dev(dev); struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc->state); unsigned long flags; @@ -405,9 +414,6 @@ static void vc4_hvs_update_dlist(struct drm_crtc *crtc) spin_unlock_irqrestore(&dev->event_lock, flags); } - HVS_WRITE(SCALER_DISPLISTX(vc4_state->assigned_channel), - vc4_state->mm.start); - spin_lock_irqsave(&vc4_crtc->irq_lock, flags); vc4_crtc->current_dlist = vc4_state->mm.start; spin_unlock_irqrestore(&vc4_crtc->irq_lock, flags); @@ -434,6 +440,7 @@ void vc4_hvs_atomic_enable(struct drm_crtc *crtc, struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); bool oneshot = vc4_crtc->feeds_txp; + vc4_hvs_install_dlist(crtc); vc4_hvs_update_dlist(crtc); vc4_hvs_init_channel(vc4, crtc, mode, oneshot); } @@ -509,8 +516,10 @@ void vc4_hvs_atomic_flush(struct drm_crtc *crtc, * If the CRTC is being disabled, there's no point in updating this * information. */ - if (crtc->state->active && old_state->active) + if (crtc->state->active && old_state->active) { + vc4_hvs_install_dlist(crtc); vc4_hvs_update_dlist(crtc); + } if (crtc->state->color_mgmt_changed) { u32 dispbkgndx = HVS_READ(SCALER_DISPBKGNDX(channel)); -- 2.35.1
[PATCH 6/8] drm/vc4: hvs: Ignore atomic_flush if we're disabled
atomic_flush will be called for each CRTC even if they aren't enabled. The whole code we have there will thus run without a properly affected channel, which can then result in all sorts of weird behaviour. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_hvs.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c index 2d540fc11357..5db125dc2358 100644 --- a/drivers/gpu/drm/vc4/vc4_hvs.c +++ b/drivers/gpu/drm/vc4/vc4_hvs.c @@ -472,6 +472,9 @@ void vc4_hvs_atomic_flush(struct drm_crtc *crtc, u32 __iomem *dlist_start = vc4->hvs->dlist + vc4_state->mm.start; u32 __iomem *dlist_next = dlist_start; + if (vc4_state->assigned_channel == VC4_HVS_CHANNEL_DISABLED) + return; + if (debug_dump_regs) { DRM_INFO("CRTC %d HVS before:\n", drm_crtc_index(crtc)); vc4_hvs_dump_state(dev); -- 2.35.1
[PATCH 7/8] drm/vc4: hvs: Use pointer to HVS in HVS_READ and HVS_WRITE macros
Those macros are really about the HVS itself, and thus its associated structure vc4_hvs, rather than the entire (virtual) vc4 device. Let's change those macros to use the hvs pointer directly, and change the calling sites accordingly. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_crtc.c | 14 +-- drivers/gpu/drm/vc4/vc4_drv.h | 16 +++ drivers/gpu/drm/vc4/vc4_hvs.c | 77 +- drivers/gpu/drm/vc4/vc4_kms.c | 5 ++- 4 files changed, 60 insertions(+), 52 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 72fadce38d32..5bb4027e479e 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -70,6 +70,7 @@ static const struct debugfs_reg32 crtc_regs[] = { static unsigned int vc4_crtc_get_cob_allocation(struct vc4_dev *vc4, unsigned int channel) { + struct vc4_hvs *hvs = vc4->hvs; u32 dispbase = HVS_READ(SCALER_DISPBASEX(channel)); /* Top/base are supposed to be 4-pixel aligned, but the * Raspberry Pi firmware fills the low bits (which are @@ -89,6 +90,7 @@ static bool vc4_crtc_get_scanout_position(struct drm_crtc *crtc, { struct drm_device *dev = crtc->dev; struct vc4_dev *vc4 = to_vc4_dev(dev); + struct vc4_hvs *hvs = vc4->hvs; struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); struct vc4_crtc_state *vc4_crtc_state = to_vc4_crtc_state(crtc->state); unsigned int cob_size; @@ -123,7 +125,7 @@ static bool vc4_crtc_get_scanout_position(struct drm_crtc *crtc, *vpos /= 2; /* Use hpos to correct for field offset in interlaced mode. */ - if (vc4_hvs_get_fifo_frame_count(dev, vc4_crtc_state->assigned_channel) % 2) + if (vc4_hvs_get_fifo_frame_count(hvs, vc4_crtc_state->assigned_channel) % 2) *hpos += mode->crtc_htotal / 2; } @@ -413,6 +415,7 @@ static void vc4_crtc_config_pv(struct drm_crtc *crtc, struct drm_encoder *encode static void require_hvs_enabled(struct drm_device *dev) { struct vc4_dev *vc4 = to_vc4_dev(dev); + struct vc4_hvs *hvs = vc4->hvs; WARN_ON_ONCE((HVS_READ(SCALER_DISPCTRL) & SCALER_DISPCTRL_ENABLE) != SCALER_DISPCTRL_ENABLE); @@ -426,6 +429,7 @@ static int vc4_crtc_disable(struct drm_crtc *crtc, struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder); struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); struct drm_device *dev = crtc->dev; + struct vc4_dev *vc4 = to_vc4_dev(dev); int ret; CRTC_WRITE(PV_V_CONTROL, @@ -455,7 +459,7 @@ static int vc4_crtc_disable(struct drm_crtc *crtc, vc4_encoder->post_crtc_disable(encoder, state); vc4_crtc_pixelvalve_reset(crtc); - vc4_hvs_stop_channel(dev, channel); + vc4_hvs_stop_channel(vc4->hvs, channel); if (vc4_encoder && vc4_encoder->post_crtc_powerdown) vc4_encoder->post_crtc_powerdown(encoder, state); @@ -481,6 +485,7 @@ static struct drm_encoder *vc4_crtc_get_encoder_by_type(struct drm_crtc *crtc, int vc4_crtc_disable_at_boot(struct drm_crtc *crtc) { struct drm_device *drm = crtc->dev; + struct vc4_dev *vc4 = to_vc4_dev(drm); struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); enum vc4_encoder_type encoder_type; const struct vc4_pv_data *pv_data; @@ -502,7 +507,7 @@ int vc4_crtc_disable_at_boot(struct drm_crtc *crtc) if (!(CRTC_READ(PV_V_CONTROL) & PV_VCONTROL_VIDEN)) return 0; - channel = vc4_hvs_get_fifo_from_output(drm, vc4_crtc->data->hvs_output); + channel = vc4_hvs_get_fifo_from_output(vc4->hvs, vc4_crtc->data->hvs_output); if (channel < 0) return 0; @@ -715,6 +720,7 @@ static void vc4_crtc_handle_page_flip(struct vc4_crtc *vc4_crtc) struct drm_crtc *crtc = &vc4_crtc->base; struct drm_device *dev = crtc->dev; struct vc4_dev *vc4 = to_vc4_dev(dev); + struct vc4_hvs *hvs = vc4->hvs; u32 chan = vc4_crtc->current_hvs_channel; unsigned long flags; @@ -733,7 +739,7 @@ static void vc4_crtc_handle_page_flip(struct vc4_crtc *vc4_crtc) * the CRTC and encoder already reconfigured, leading to * underruns. This can be seen when reconfiguring the CRTC. */ - vc4_hvs_unmask_underrun(dev, chan); + vc4_hvs_unmask_underrun(hvs, chan); } spin_unlock(&vc4_crtc->irq_lock); spin_unlock_irqrestore(&dev->event_lock, flags); diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 801da3e8ebdb..15e0c2ac3940 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -574,8 +574,8 @@ to_vc4_crtc_state(struct drm_crtc_state *crtc_state) #define V3D_READ(offset) readl(vc4->v3d->regs + offset) #define V3D_WRITE(offset, val) writel(va
[PATCH 8/8] drm/vc4: hvs: Defer dlist slots deallocation
During normal operations, the cursor position update is done through an asynchronous plane update, which on the vc4 driver basically just modifies the right dlist word to move the plane to the new coordinates. However, when we have the overscan margins setup, we fall back to a regular commit when we are next to the edges. And since that commit happens to be on a cursor plane, it's considered a legacy cursor update by KMS. The main difference it makes is that it won't wait for its completion (ie, next vblank) before returning. This means if we have multiple commits happening in rapid succession, we can have several of them happening before the next vblank. In parallel, our dlist allocation is tied to a CRTC state, and each time we do a commit we end up with a new CRTC state, with the previous one being freed. This means that we free our previous dlist entry (but don't clear it though) every time a new one is being committed. Now, if we were to have two commits happening before the next vblank, we could end up freeing reusing the same dlist entries before the next vblank. Indeed, we would start from an initial state taking, for example, the dlist entries 10 to 20, then start a commit taking the entries 20 to 30 and setting the dlist pointer to 20, and freeing the dlist entries 10 to 20. However, since we haven't reach vblank yet, the HVS is still using the entries 10 to 20. If we were to make a new commit now, chances are the allocator are going to give the 10 to 20 entries back, and we would change their content to match the new state. If vblank hasn't happened yet, we just corrupted the active dlist entries. A first attempt to solve this was made by creating an intermediate dlist buffer to store the current (ie, as of the last commit) dlist content, that we would update each time the HVS is done with a frame. However, if the interrupt handler missed the vblank window, we would end up copying our intermediate dlist to the hardware one during the composition, essentially creating the same issue. Since making sure that our interrupt handler runs within a fixed, constrained, time window would require to make Linux a real-time kernel, this seems a bit out of scope. Instead, we can work around our original issue by keeping the dlist slots allocation longer. That way, we won't reuse a dlist slot while it's still in flight. In order to achieve this, instead of freeing the dlist slot when its associated CRTC state is destroyed, we'll queue it in a list. A naive implementation would free the buffers in that queue when we get our end of frame interrupt. However, there's still a race since, just like in the shadow dlist case, we don't control when the handler for that interrupt is going to run. Thus, we can end up with a commit adding an old dlist allocation to our queue during the window between our actual interrupt and when our handler will run. And since that buffer is still being used for the composition of the current frame, we can't free it right away, exposing us to the original bug. Fortunately for us, the hardware provides a frame counter that is increased each time the first line of a frame is being generated. Associating the frame counter the image is supposed to go away to the allocation, and then only deallocate buffers that have a counter below or equal to the one we see when the deallocation code should prevent the above race from occuring. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_crtc.c | 10 +- drivers/gpu/drm/vc4/vc4_drv.h | 15 ++- drivers/gpu/drm/vc4/vc4_hvs.c | 181 ++--- drivers/gpu/drm/vc4/vc4_regs.h | 1 + 4 files changed, 184 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 5bb4027e479e..ed5fc400b66d 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -920,14 +920,8 @@ void vc4_crtc_destroy_state(struct drm_crtc *crtc, struct vc4_dev *vc4 = to_vc4_dev(crtc->dev); struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(state); - if (drm_mm_node_allocated(&vc4_state->mm)) { - unsigned long flags; - - spin_lock_irqsave(&vc4->hvs->mm_lock, flags); - drm_mm_remove_node(&vc4_state->mm); - spin_unlock_irqrestore(&vc4->hvs->mm_lock, flags); - - } + vc4_hvs_mark_dlist_entry_stale(vc4->hvs, vc4_state->mm); + vc4_state->mm = NULL; drm_atomic_helper_crtc_destroy_state(crtc, state); } diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 15e0c2ac3940..4d155af0262d 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -330,6 +330,9 @@ struct vc4_hvs { struct drm_mm lbm_mm; spinlock_t mm_lock; + struct list_head stale_dlist_entries; + struct work_struct free_dlist_work; + struct drm_mm_node mitchell_netravali_filter; struct debugfs_regset32 reg
Re: [PATCH v10 1/4] MIPS: Loongson64: dts: update the display controller device node
On 2022/2/21 17:19, Sergei Shtylyov wrote: On 2/20/22 5:55 PM, Sui Jingfeng wrote: From: suijingfeng The display controller is a pci device, its PCI vendor id is 0x0014 its PCI device id is 0x7a06. 1) In order to let the driver to know which chip the DC is contained in, the compatible string of the display controller is updated according to the chip's name. 2) Add display controller device node for ls2k1000 SoC Reported-by: Krzysztof Kozlowski Signed-off-by: suijingfeng Signed-off-by: Sui Jingfeng <15330273...@189.cn> --- arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi | 8 arch/mips/boot/dts/loongson/ls7a-pch.dtsi | 7 ++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi index 768cf2abcea3..af9cda540f9e 100644 --- a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi +++ b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi @@ -209,6 +209,14 @@ gpu@5,0 { interrupt-parent = <&liointc0>; }; + lsdc: display-controller@6,0 { Shouldn't the node name just be "display", according to the section 2.2.2 of the DT spec? [...] diff --git a/arch/mips/boot/dts/loongson/ls7a-pch.dtsi b/arch/mips/boot/dts/loongson/ls7a-pch.dtsi index 2f45fce2cdc4..ec35ea9b2fe8 100644 --- a/arch/mips/boot/dts/loongson/ls7a-pch.dtsi +++ b/arch/mips/boot/dts/loongson/ls7a-pch.dtsi @@ -160,11 +160,8 @@ gpu@6,0 { interrupt-parent = <&pic>; }; - dc@6,1 { - compatible = "pci0014,7a06.0", - "pci0014,7a06", - "pciclass03", - "pciclass0300"; + lsdc: display-controller@6,1 { Same here... [...] MBR, Sergey Display sounds like a panel or monitor, while we are the device that driven the display device. Running find . -name "*.dtsi" -type f | xargs grep "display-controller" at drm-tip/arch/ directory show that there are a number of vendors using display controller as theirnode name, for example Atmel and STM32. ./arm/boot/dts/pxa3xx.dtsi: gcu: display-controller@5400 { ./arm/boot/dts/at91sam9n12.dtsi: hlcdc-display-controller { ./arm/boot/dts/at91sam9n12.dtsi:compatible = "atmel,hlcdc-display-controller"; ./arm/boot/dts/at91-dvk_su60_somc_lcm.dtsi: hlcdc-display-controller { ./arm/boot/dts/stm32h743.dtsi: ltdc: display-controller@50001000 { ./arm/boot/dts/stm32mp151.dtsi: ltdc: display-controller@5a001000 { ./arm/boot/dts/at91sam9x5dm.dtsi: hlcdc-display-controller { ./arm/boot/dts/gemini.dtsi: display-controller@6a00 { ./arm/boot/dts/stm32f429.dtsi: ltdc: display-controller@40016800 { ./arm/boot/dts/at91sam9x5_lcd.dtsi: hlcdc-display-controller { ./arm/boot/dts/at91sam9x5_lcd.dtsi: compatible = "atmel,hlcdc-display-controller"; ./arm/boot/dts/sama5d2.dtsi: hlcdc-display-controller { ./arm/boot/dts/sama5d2.dtsi:compatible = "atmel,hlcdc-display-controller"; ./arm/boot/dts/sama5d4.dtsi: hlcdc-display-controller { ./arm/boot/dts/sama5d4.dtsi:compatible = "atmel,hlcdc-display-controller"; ./arm/boot/dts/sama5d3_lcd.dtsi: hlcdc-display-controller { ./arm/boot/dts/sama5d3_lcd.dtsi:compatible = "atmel,hlcdc-display-controller"; ./arm/boot/dts/sam9x60.dtsi: hlcdc-display-controller { ./arm/boot/dts/sam9x60.dtsi:compatible = "atmel,hlcdc-display-controller"; ./arm/boot/dts/at91-sama5d4_ma5d4evk.dts: hlcdc-display-controller { ./arm/boot/dts/at91sam9n12ek.dts: hlcdc-display-controller { ./arm/boot/dts/am335x-pdu001.dts: display-controller@0 { ./arm/boot/dts/at91-nattis-2-natte-2.dts: hlcdc-display-controller { ./arm/boot/dts/gemini-dlink-dir-685.dts: display-controller@6a00 {
Re: [PATCH 1/2] dt-bindings: display: imx: Add fsl,imx21-lcdc docs
Hi Rob, On Thu, Feb 10, 2022 at 06:54:13PM +0100, Lucas Stach wrote: > Am Dienstag, dem 01.02.2022 um 11:35 -0600 schrieb Rob Herring: > > On Fri, Jan 28, 2022 at 06:58:29PM +0100, Uwe Kleine-König wrote: > > > On Fri, Jan 28, 2022 at 07:04:10AM -0600, Rob Herring wrote: > > > > On Fri, Jan 28, 2022 at 4:59 AM Uwe Kleine-König > > > > wrote: > > > > > > > > > > From: Marian Cichy > > > > > > > > > > This files documents the device tree for the new imx21-lcdc DRM > > > > > driver. > > > > > > > > No, bindings document h/w and the h/w has not changed. We already have > > > > a binding for the LCDC. > > > > > > Just to be sure we're talking about the same thing: You're refering to > > > Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt, right? > > > > Looks right... > > > > > I'm unsure what to do now. Should the two different bindings just be > > > described in the same file? Should I stick to fsl,imx21-fb even for the > > > new binding? (The hardware unit is named LCDC, so the name chosen here > > > is the better one.) Please advise. > > > > Yes, the name is unfortunate, but it should be 1 binding, 1 file, and > > unchanged (unless you want to add new optional properties). > > The old FB driver binding is pretty insane. Except the reg and > interrupt properties it is very confused about things. It exposes > internal implementation details (like specifying verbatim register > settings in the DT) and other properties are just misplaced, like the > lcd-supply property that controls the panel power supply. > > I really don't think that trying to stay backwards compatible here is a > win for anyone. Anyone willing to switch their systems running on a 15 > year old SoC to the new DRM driver probably doesn't mind if they have > to modify the DTS to make it work. Can we please let the FB driver die > in peace and have a clean slate driver/binding for the DRM driver? Does this feedback change anything on your side? My expectation is that the graphics people will be happy about every fb driver being replaced by a DRM driver and there will be hardly any incentive to add a layer over the DRM driver to make it honor the old fb binding. Assume I convert the old binding to yaml and then add the newly supported binding to that using a big oneOf, would that be an acceptable compromise? Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [PATCH v10 1/4] MIPS: Loongson64: dts: update the display controller device node
On 2022/2/21 18:01, Krzysztof Kozlowski wrote: On 21/02/2022 10:19, Sergei Shtylyov wrote: On 2/20/22 5:55 PM, Sui Jingfeng wrote: From: suijingfeng The display controller is a pci device, its PCI vendor id is 0x0014 its PCI device id is 0x7a06. 1) In order to let the driver to know which chip the DC is contained in, the compatible string of the display controller is updated according to the chip's name. 2) Add display controller device node for ls2k1000 SoC Reported-by: Krzysztof Kozlowski Signed-off-by: suijingfeng Signed-off-by: Sui Jingfeng <15330273...@189.cn> --- arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi | 8 arch/mips/boot/dts/loongson/ls7a-pch.dtsi | 7 ++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi index 768cf2abcea3..af9cda540f9e 100644 --- a/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi +++ b/arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi @@ -209,6 +209,14 @@ gpu@5,0 { interrupt-parent = <&liointc0>; }; + lsdc: display-controller@6,0 { Shouldn't the node name just be "display", according to the section 2.2.2 of the DT spec? lcd-controller, led-controller. As I understood from the bindings, this is not physical device displaying something (like a panel) but rather a device controlling such panel. Therefore display-controller feels appropriate. Best regards, Krzysztof Extremely correct.
Re: Regression from 3c196f056666 ("drm/amdgpu: always reset the asic in suspend (v2)") on suspend?
On Mon, Feb 21, 2022 at 3:29 AM Eric Valette wrote: > > On 20/02/2022 16:48, Dominique Dumont wrote: > > On Monday, 14 February 2022 22:52:27 CET Alex Deucher wrote: > >> Does the system actually suspend? > > > > Not really. The screens looks like it's going to suspend, but it does come > > back after 10s or so. The light mounted in the middle of the power button > > does > > not switch off. > > > As I have a very similar problem and also commented on the original > debian bug report > (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1005005), I will add > some information here on another amd only laptop (renoir AMD Ryzen 7 > 4800H with Radeon Graphics + Radeon RX 5500/5500M / Pro 5500M). > > For me the suspend works once, but after the first resume (I do know > know if it is in the suspend path or the resume path I see a RIP in the > dmesg (see aditional info in debian bug)) and later suspend do not > work: It only go to the kde login screen. > > I was unable due to network connectivity to do a full bisect but tested > with the patch I had on my laptop: > > 5.10.101 works, 5.10 from debian works > 5.11 works > 5.12 works > 5.13 suspend works but when resuming the PC is dead I have to reboot > 5.14 seems to work but looking at dmesg it is full of RIP messages at > various places. > 5.15.24 is a described 5.15 from debian is behaving identically > 5.16 from debian is behaving identically. > > >> Is this system S0i3 or regular S3? > > For me it is real S3. > > The proposed patch is intended for INTEl + intel gpu + amdgpu but I have > dual amd GPU. It doesn't really matter what the platform is, it could still potentially help on your system, it depends on the bios implementation for your platform and how it handles suspend. You can try the patch, but I don't think you are hitting the same issue. I bisect would be helpful in your case. Alex
Re: [PATCH v8 04/19] video/hdmi: Add audio_infoframe packing for DP
Il 18/02/22 15:54, Guillaume Ranquet ha scritto: From: Markus Schneider-Pargmann Similar to HDMI, DP uses audio infoframes as well which are structured very similar to the HDMI ones. This patch adds a helper function to pack the HDMI audio infoframe for DP, called hdmi_audio_infoframe_pack_for_dp(). hdmi_audio_infoframe_pack_only() is split into two parts. One of them packs the payload only and can be used for HDMI and DP. Signed-off-by: Markus Schneider-Pargmann Signed-off-by: Guillaume Ranquet --- drivers/video/hdmi.c| 83 - include/drm/drm_dp_helper.h | 2 + include/linux/hdmi.h| 7 +++- 3 files changed, 72 insertions(+), 20 deletions(-) diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c index 947be761dfa40..63e74d9fd210e 100644 --- a/drivers/video/hdmi.c +++ b/drivers/video/hdmi.c @@ -21,6 +21,7 @@ * DEALINGS IN THE SOFTWARE. */ +#include #include #include #include @@ -381,12 +382,34 @@ static int hdmi_audio_infoframe_check_only(const struct hdmi_audio_infoframe *fr * * Returns 0 on success or a negative error code on failure. */ -int hdmi_audio_infoframe_check(struct hdmi_audio_infoframe *frame) +int hdmi_audio_infoframe_check(const struct hdmi_audio_infoframe *frame) { return hdmi_audio_infoframe_check_only(frame); } EXPORT_SYMBOL(hdmi_audio_infoframe_check); +static void +hdmi_audio_infoframe_pack_payload(const struct hdmi_audio_infoframe *frame, + u8 *buffer) +{ + u8 channels; + + if (frame->channels >= 2) + channels = frame->channels - 1; + else + channels = 0; + + buffer[0] = ((frame->coding_type & 0xf) << 4) | (channels & 0x7); + buffer[1] = ((frame->sample_frequency & 0x7) << 2) | +(frame->sample_size & 0x3); + buffer[2] = frame->coding_type_ext & 0x1f; + buffer[3] = frame->channel_allocation; + buffer[4] = (frame->level_shift_value & 0xf) << 3; + + if (frame->downmix_inhibit) + buffer[4] |= BIT(7); +} + /** * hdmi_audio_infoframe_pack_only() - write HDMI audio infoframe to binary buffer * @frame: HDMI audio infoframe @@ -404,7 +427,6 @@ EXPORT_SYMBOL(hdmi_audio_infoframe_check); ssize_t hdmi_audio_infoframe_pack_only(const struct hdmi_audio_infoframe *frame, void *buffer, size_t size) { - unsigned char channels; u8 *ptr = buffer; size_t length; int ret; @@ -420,28 +442,13 @@ ssize_t hdmi_audio_infoframe_pack_only(const struct hdmi_audio_infoframe *frame, memset(buffer, 0, size); - if (frame->channels >= 2) - channels = frame->channels - 1; - else - channels = 0; - ptr[0] = frame->type; ptr[1] = frame->version; ptr[2] = frame->length; ptr[3] = 0; /* checksum */ - /* start infoframe payload */ - ptr += HDMI_INFOFRAME_HEADER_SIZE; - - ptr[0] = ((frame->coding_type & 0xf) << 4) | (channels & 0x7); - ptr[1] = ((frame->sample_frequency & 0x7) << 2) | -(frame->sample_size & 0x3); - ptr[2] = frame->coding_type_ext & 0x1f; - ptr[3] = frame->channel_allocation; - ptr[4] = (frame->level_shift_value & 0xf) << 3; - - if (frame->downmix_inhibit) - ptr[4] |= BIT(7); + hdmi_audio_infoframe_pack_payload(frame, + ptr + HDMI_INFOFRAME_HEADER_SIZE); hdmi_infoframe_set_checksum(buffer, length); @@ -479,6 +486,44 @@ ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame, } EXPORT_SYMBOL(hdmi_audio_infoframe_pack); +/** + * hdmi_audio_infoframe_pack_for_dp - Pack a HDMI Audio infoframe for + *displayport This fits in one line (82 cols is ok)... but, anyway, please capitalize D and P in the DisplayPort word. + * + * @frame HDMI Audio infoframe You're almost there! You missed a colon after each description. Also, I personally like seeing indented descriptions as, in my opinion, this enhances human readability, as it's a bit more pleasing to the eye... but it's not a requirement, so you be the judge. * @frame: HDMI Audio infoframe * @sdp:Secondary data packet.. * @dp_version: DisplayPort version.. Please fix. + * @sdp secondary data packet for display port. This is filled with the + * appropriate data + * @dp_version Display Port version to be encoded in the header + * + * Packs a HDMI Audio Infoframe to be sent over Display Port. This function + * fills the secondary data packet to be used for Display Port. + * + * Return: Number of total written bytes or a negative errno on failure. + */ +ssize_t +hdmi_audio_infoframe_pack_for_dp(const struct hdmi_audio_infoframe *frame, +struct dp_sdp *sdp, u8 dp_version) +{ + int ret; + + ret = hdmi_audi
Re: [PATCH v8 03/19] drm/edid: Add cea_sad helpers for freq/length
Il 18/02/22 15:54, Guillaume Ranquet ha scritto: From: Markus Schneider-Pargmann This patch adds two helper functions that extract the frequency and word length from a struct cea_sad. For these helper functions new defines are added that help translate the 'freq' and 'byte2' fields into real numbers. Signed-off-by: Markus Schneider-Pargmann Signed-off-by: Guillaume Ranquet --- drivers/gpu/drm/drm_edid.c | 74 ++ include/drm/drm_edid.h | 18 -- 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 12893e7be89bb..500279a82167a 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4747,6 +4747,80 @@ int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb) } EXPORT_SYMBOL(drm_edid_to_speaker_allocation); +/** + * drm_cea_sad_get_sample_rate - Extract the sample rate from cea_sad + * @sad: Pointer to the cea_sad struct + * + * Extracts the cea_sad frequency field and returns the sample rate in Hz. + * + * Return: Sample rate in Hz or a negative errno if parsing failed. + */ +int drm_cea_sad_get_sample_rate(const struct cea_sad *sad) +{ + switch (sad->freq) { + case DRM_CEA_SAD_FREQ_32KHZ: + return 32000; + case DRM_CEA_SAD_FREQ_44KHZ: + return 44100; + case DRM_CEA_SAD_FREQ_48KHZ: + return 48000; + case DRM_CEA_SAD_FREQ_88KHZ: + return 88200; + case DRM_CEA_SAD_FREQ_96KHZ: + return 96000; + case DRM_CEA_SAD_FREQ_176KHZ: + return 176400; + case DRM_CEA_SAD_FREQ_192KHZ: + return 192000; + default: + return -EINVAL; + } +} +EXPORT_SYMBOL(drm_cea_sad_get_sample_rate); + +static bool drm_cea_sad_is_uncompressed(const struct cea_sad *sad) +{ + switch (sad->format) { + case HDMI_AUDIO_CODING_TYPE_STREAM: + case HDMI_AUDIO_CODING_TYPE_PCM: + return true; + default: + return false; + } +} + +/** + * drm_cea_sad_get_uncompressed_word_length - Extract word length + * @sad: Pointer to the cea_sad struct + * + * Extracts the cea_sad byte2 field and returns the word length for an + * uncompressed stream. + * + * Note: This function may only be called for uncompressed audio. + * + * Return: Word length in bits or a negative errno if parsing failed. + */ +int drm_cea_sad_get_uncompressed_word_length(const struct cea_sad *sad) +{ + if (!drm_cea_sad_is_uncompressed(sad)) { + DRM_WARN("Unable to get the uncompressed word length for a compressed format: %u\n", +sad->format); + return -EINVAL; + } + + switch (sad->byte2) { + case DRM_CEA_SAD_UNCOMPRESSED_WORD_16BIT: + return 16; + case DRM_CEA_SAD_UNCOMPRESSED_WORD_20BIT: + return 20; + case DRM_CEA_SAD_UNCOMPRESSED_WORD_24BIT: + return 24; + default: + return -EINVAL; + } +} +EXPORT_SYMBOL(drm_cea_sad_get_uncompressed_word_length); + /** * drm_av_sync_delay - compute the HDMI/DP sink audio-video sync delay * @connector: connector associated with the HDMI/DP sink diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 18f6c700f6d02..a30452b313979 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -361,12 +361,24 @@ struct edid { /* Short Audio Descriptor */ struct cea_sad { - u8 format; + u8 format; /* See HDMI_AUDIO_CODING_TYPE_* */ Hello Guillaume, since you're adding comments to all the rest of the struct members, I think that a small effort to instead convert this to kerneldoc is totally worth it. Can you please do that? Thanks, Angelo u8 channels; /* max number of channels - 1 */ - u8 freq; + u8 freq; /* See CEA_SAD_FREQ_* */ u8 byte2; /* meaning depends on format */ }; +#define DRM_CEA_SAD_FREQ_32KHZ BIT(0) +#define DRM_CEA_SAD_FREQ_44KHZ BIT(1) +#define DRM_CEA_SAD_FREQ_48KHZ BIT(2) +#define DRM_CEA_SAD_FREQ_88KHZ BIT(3) +#define DRM_CEA_SAD_FREQ_96KHZ BIT(4) +#define DRM_CEA_SAD_FREQ_176KHZ BIT(5) +#define DRM_CEA_SAD_FREQ_192KHZ BIT(6) + +#define DRM_CEA_SAD_UNCOMPRESSED_WORD_16BIT BIT(0) +#define DRM_CEA_SAD_UNCOMPRESSED_WORD_20BIT BIT(1) +#define DRM_CEA_SAD_UNCOMPRESSED_WORD_24BIT BIT(2) + struct drm_encoder; struct drm_connector; struct drm_connector_state; @@ -374,6 +386,8 @@ struct drm_display_mode; int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads); int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb); +int drm_cea_sad_get_sample_rate(const struct cea_sad *sad); +int drm_cea_sad_get_uncompressed_word_length(const struct cea_sad *sad); int drm_av_sync_delay(struct drm_connector *connector, const struct drm_display_mode *mode);
Re: [PATCH v8 06/19] drm/mediatek: dpi: implement a CK/DE pol toggle in board config
Il 18/02/22 15:54, Guillaume Ranquet ha scritto: Adds a bit of flexibility to support boards without CK/DE pol support Signed-off-by: Guillaume Ranquet s/board/SoC/g After the change, Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH v8 05/19] drm/mediatek: dpi: move dpi limits to board config
Il 18/02/22 15:54, Guillaume Ranquet ha scritto: Add flexibility by moving the dpi limits to the board config s/board/SoC/g After the change, Reviewed-by: AngeloGioacchino Del Regno Signed-off-by: Guillaume Ranquet
Re: [PATCH v8 13/19] drm/mediatek: dpi: Add dpintf support
Il 18/02/22 15:54, Guillaume Ranquet ha scritto: dpintf is the displayport interface hardware unit. This unit is similar to dpi and can reuse most of the code. This patch adds support for mt8195-dpintf to this dpi driver. Main differences are: - Some features/functional components are not available for dpintf which are now excluded from code execution once is_dpintf is set - dpintf can and needs to choose between different clockdividers based on the clockspeed. This is done by choosing a different clock parent. - There are two additional clocks that need to be managed. These are only set for dpintf and will be set to NULL if not supplied. The clk_* calls handle these as normal clocks then. - Some register contents differ slightly between the two components. To work around this I added register bits/masks with a DPINTF_ prefix and use them where different. Based on a separate driver for dpintf created by Jason-JH.Lin . Signed-off-by: Markus Schneider-Pargmann Signed-off-by: Guillaume Ranquet --- drivers/gpu/drm/mediatek/mtk_dpi.c | 164 +--- drivers/gpu/drm/mediatek/mtk_dpi_regs.h | 38 + drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 8 + drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 + drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 +- include/linux/soc/mediatek/mtk-mmsys.h | 2 + 6 files changed, 198 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c index be99399faf1bb..c5639ada868f8 100644 --- a/drivers/gpu/drm/mediatek/mtk_dpi.c +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c @@ -63,6 +63,14 @@ enum mtk_dpi_out_color_format { MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL }; +enum TVDPLL_CLK { + TVDPLL_PLL = 0, + TVDPLL_D2 = 2, + TVDPLL_D4 = 4, + TVDPLL_D8 = 8, + TVDPLL_D16 = 16, +}; + struct mtk_dpi { struct drm_encoder encoder; struct drm_bridge bridge; @@ -71,8 +79,10 @@ struct mtk_dpi { void __iomem *regs; struct device *dev; struct clk *engine_clk; + struct clk *dpi_ck_cg; struct clk *pixel_clk; struct clk *tvd_clk; + struct clk *pclk_src[5]; int irq; struct drm_display_mode mode; const struct mtk_dpi_conf *conf; @@ -126,6 +136,7 @@ struct mtk_dpi_conf { const u32 *output_fmts; u32 num_output_fmts; bool is_ck_de_pol; + bool is_dpintf; bool swap_input_support; // Mask used for HWIDTH, HPORCH, VSYNC_WIDTH and VSYNC_PORCH (no shift) u32 dimension_mask; @@ -384,6 +395,25 @@ static void mtk_dpi_config_disable_edge(struct mtk_dpi *dpi) mtk_dpi_mask(dpi, dpi->conf->reg_h_fre_con, 0, EDGE_SEL_EN); } +static void mtk_dpi_matrix_sel(struct mtk_dpi *dpi, enum mtk_dpi_out_color_format format) +{ + u32 matrix_sel = 0; + + switch (format) { + case MTK_DPI_COLOR_FORMAT_YCBCR_422: + case MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL: + case MTK_DPI_COLOR_FORMAT_YCBCR_444: + case MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL: + case MTK_DPI_COLOR_FORMAT_XV_YCC: + if (dpi->mode.hdisplay <= 720) + matrix_sel = 0x2; + break; + default: + break; + } + mtk_dpi_mask(dpi, DPI_MATRIX_SET, matrix_sel, INT_MATRIX_SEL_MASK); +} + static void mtk_dpi_config_color_format(struct mtk_dpi *dpi, enum mtk_dpi_out_color_format format) { @@ -391,6 +421,7 @@ static void mtk_dpi_config_color_format(struct mtk_dpi *dpi, (format == MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL)) { mtk_dpi_config_yuv422_enable(dpi, false); mtk_dpi_config_csc_enable(dpi, true); + mtk_dpi_matrix_sel(dpi, format); if (dpi->conf->swap_input_support) mtk_dpi_config_swap_input(dpi, false); mtk_dpi_config_channel_swap(dpi, MTK_DPI_OUT_CHANNEL_SWAP_BGR); @@ -398,6 +429,7 @@ static void mtk_dpi_config_color_format(struct mtk_dpi *dpi, (format == MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL)) { mtk_dpi_config_yuv422_enable(dpi, true); mtk_dpi_config_csc_enable(dpi, true); + mtk_dpi_matrix_sel(dpi, format); if (dpi->conf->swap_input_support) mtk_dpi_config_swap_input(dpi, true); mtk_dpi_config_channel_swap(dpi, MTK_DPI_OUT_CHANNEL_SWAP_RGB); @@ -438,6 +470,8 @@ static void mtk_dpi_power_off(struct mtk_dpi *dpi) mtk_dpi_disable(dpi); clk_disable_unprepare(dpi->pixel_clk); clk_disable_unprepare(dpi->engine_clk); + clk_disable_unprepare(dpi->dpi_ck_cg); + clk_disable_unprepare(dpi->tvd_clk); } static int mtk_dpi_power_on(struct mtk_dpi *dpi) @@ -447,12 +481,24 @@ static int mtk_dpi_power_on(struct mtk_dpi *dpi)
Re: [PATCH v8 08/19] drm/mediatek: dpi: move dimension mask to board config
Il 18/02/22 15:54, Guillaume Ranquet ha scritto: Add flexibility by moving the dimension mask to the board config s/board/SoC/g Signed-off-by: Guillaume Ranquet --- drivers/gpu/drm/mediatek/mtk_dpi.c | 26 -- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c index 454f8563efae4..8ca3455ed64ee 100644 --- a/drivers/gpu/drm/mediatek/mtk_dpi.c +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c @@ -127,6 +127,8 @@ struct mtk_dpi_conf { u32 num_output_fmts; bool is_ck_de_pol; bool swap_input_support; + // Mask used for HWIDTH, HPORCH, VSYNC_WIDTH and VSYNC_PORCH (no shift) Also, C-style comments, please. After the change, Reviewed-by: AngeloGioacchino Del Regno + u32 dimension_mask; const struct mtk_dpi_yc_limit *limit; };
Re: [PATCH v8 09/19] drm/mediatek: dpi: move dimension_mask to board config
Il 18/02/22 15:54, Guillaume Ranquet ha scritto: Add flexibility by moving the dimension mask to board config You should really fix both title and description for this commit, as this is moving hvsize_mask, not dimension_mask. Also, s/board/SoC/g. Signed-off-by: Guillaume Ranquet --- drivers/gpu/drm/mediatek/mtk_dpi.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c index 8ca3455ed64ee..0d3acd08ea358 100644 --- a/drivers/gpu/drm/mediatek/mtk_dpi.c +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c @@ -129,6 +129,8 @@ struct mtk_dpi_conf { bool swap_input_support; // Mask used for HWIDTH, HPORCH, VSYNC_WIDTH and VSYNC_PORCH (no shift) u32 dimension_mask; + // Mask used for HSIZE and VSIZE (no shift) C-style comments please + u32 hvsize_mask; const struct mtk_dpi_yc_limit *limit; };
Re: [PATCH v8 10/19] drm/mediatek: dpi: move swap_shift to board config
Il 18/02/22 15:54, Guillaume Ranquet ha scritto: Add flexibility by moving the swap shift value to board config Same board->SoC as all the other commits.. Signed-off-by: Guillaume Ranquet --- drivers/gpu/drm/mediatek/mtk_dpi.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c index 0d3acd08ea358..ec221e24e0fee 100644 --- a/drivers/gpu/drm/mediatek/mtk_dpi.c +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c @@ -131,6 +131,7 @@ struct mtk_dpi_conf { u32 dimension_mask; // Mask used for HSIZE and VSIZE (no shift) u32 hvsize_mask; + u32 channel_swap_shift; const struct mtk_dpi_yc_limit *limit; }; @@ -349,7 +350,8 @@ static void mtk_dpi_config_channel_swap(struct mtk_dpi *dpi, break; } - mtk_dpi_mask(dpi, DPI_OUTPUT_SETTING, val << CH_SWAP, CH_SWAP_MASK); + mtk_dpi_mask(dpi, DPI_OUTPUT_SETTING, val << CH_SWAP, +CH_SWAP_MASK << dpi->conf->channel_swap_shift); I would say that this should be instead: mtk_dpi_mask(dpi, DPI_OUTPUT_SETTING, val << dpi->conf->channel_swap_shift, CH_SWAP_MASK); } static void mtk_dpi_config_yuv422_enable(struct mtk_dpi *dpi, bool enable) @@ -821,6 +823,7 @@ static const struct mtk_dpi_conf mt8173_conf = { .swap_input_support = true, .dimension_mask = HPW_MASK, .hvsize_mask = HSIZE_MASK, + .channel_swap_shift = CH_SWAP, .limit = &mtk_dpi_limit, }; @@ -835,6 +838,7 @@ static const struct mtk_dpi_conf mt2701_conf = { .swap_input_support = true, .dimension_mask = HPW_MASK, .hvsize_mask = HSIZE_MASK, + .channel_swap_shift = CH_SWAP, .limit = &mtk_dpi_limit, }; @@ -848,6 +852,7 @@ static const struct mtk_dpi_conf mt8183_conf = { .swap_input_support = true, .dimension_mask = HPW_MASK, .hvsize_mask = HSIZE_MASK, + .channel_swap_shift = CH_SWAP, .limit = &mtk_dpi_limit, }; @@ -861,6 +866,7 @@ static const struct mtk_dpi_conf mt8192_conf = { .swap_input_support = true, .dimension_mask = HPW_MASK, .hvsize_mask = HSIZE_MASK, + .channel_swap_shift = CH_SWAP, .limit = &mtk_dpi_limit, };