[PATCH] drm/amd/display: remove unnecessary conditional operators
From: Ye Guojin
Since the variables named is_end_of_payload and hpd_status are already
bool type, the ?: conditional operator is unnecessary any more.
Clean them up here.
Reported-by: Zeal Robot
Signed-off-by: Ye Guojin
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +--
drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
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 43e983e42c0f..752a79724ce1 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -784,8 +784,7 @@ static void dm_dmub_outbox1_low_irq(void *interrupt_params)
plink =
adev->dm.dc->links[notify.link_index];
if (plink) {
plink->hpd_status =
- notify.hpd_status ==
- DP_HPD_PLUG ? true :
false;
+ notify.hpd_status ==
DP_HPD_PLUG;
}
}
queue_work(adev->dm.delayed_hpd_wq,
&dmub_hpd_wrk->handle_hpd_work);
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
index 60539b1f2a80..24dc662ec3e4 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
@@ -626,7 +626,7 @@ bool dal_ddc_submit_aux_command(struct ddc_service *ddc,
do {
struct aux_payload current_payload;
bool is_end_of_payload = (retrieved +
DEFAULT_AUX_MAX_DATA_SIZE) >=
- payload->length ? true : false;
+ payload->length;
uint32_t payload_length = is_end_of_payload ?
payload->length - retrieved :
DEFAULT_AUX_MAX_DATA_SIZE;
--
2.25.1
Re: [PATCH] drm: Grab reference of connector before return connector from sun4i_tcon_get_connector
On Tue, 02 Nov 2021, He Ying wrote: > 在 2021/11/1 21:02, Jani Nikula 写道: >> On Mon, 01 Nov 2021, He Ying wrote: >>> From the comments of drm_for_each_connector_iter(), we know >>> that "connector is only valid within the list body, if you >>> want to use connector after calling drm_connector_list_iter_end() >>> then you need to grab your own reference first using >>> drm_connector_get()". So fix the wrong use of connector >>> according to the comments and then call drm_connector_put() >>> after using connector finishes. >>> >>> Signed-off-by: He Ying >> Please use "drm/sun4i:" subject prefix for sun4i patches. > > OK. I'll send a V2 later. By the way, may I have your suggestions about > this patch's content. Sorry, I only looked here because of the subject prefix, and would have skipped it completely with a drm/sun4i prefix. ;) BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH v3] backlight: lp855x: Switch to atomic PWM API
Hi Maíra,
On Sat, Oct 30, 2021 at 3:35 PM Maíra Canal wrote:
> Remove legacy PWM interface (pwm_config, pwm_enable, pwm_disable) and
> replace it for the atomic PWM API.
>
> Signed-off-by: Maíra Canal
Thanks for your patch!
> --- a/drivers/video/backlight/lp855x_bl.c
> +++ b/drivers/video/backlight/lp855x_bl.c
> @@ -233,9 +233,8 @@ static int lp855x_configure(struct lp855x *lp)
>
> static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
> {
> - unsigned int period = lp->pdata->period_ns;
> - unsigned int duty = br * period / max_br;
> - struct pwm_device *pwm;
> + struct pwm_device *pwm = NULL;
> + struct pwm_state state;
>
> /* request pwm device with the consumer name */
> if (!lp->pwm) {
> @@ -245,18 +244,16 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br,
> int max_br)
>
> lp->pwm = pwm;
>
> - /*
> -* FIXME: pwm_apply_args() should be removed when switching to
> -* the atomic PWM API.
> -*/
> - pwm_apply_args(pwm);
> + pwm_init_state(lp->pwm, &state);
> + state.period = lp->pdata->period_ns;
> }
>
> - pwm_config(lp->pwm, duty, period);
> - if (duty)
> - pwm_enable(lp->pwm);
> - else
> - pwm_disable(lp->pwm);
> + pwm_get_state(lp->pwm, &state);
> +
> + state.duty_cycle = br * state.period / max_br;
Above is a 64-by-32 division, which should not be open-coded, as
that will cause link failures on 32-bit platform (cfr. "undefined
reference to `__udivdi3'", as reported by the kernel test robot).
Please use div_u64() instead.
> + state.enabled = state.duty_cycle;
> +
> + pwm_apply_state(lp->pwm, &state);
> }
>
> static int lp855x_bl_update_status(struct backlight_device *bl)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
RE: [PATCH v2] drm/dp: Fix aux->transfer NULL pointer dereference on drm_dp_dpcd_access
On Tue, 02 Nov 2021, "Yuan, Perry" wrote: > [AMD Official Use Only] > > Hi Jani: > Thanks for your comments. > >> -Original Message- >> From: Jani Nikula >> Sent: Monday, November 1, 2021 9:07 PM >> To: Yuan, Perry ; Maarten Lankhorst >> ; Maxime Ripard ; >> Thomas Zimmermann ; David Airlie ; >> Daniel Vetter >> Cc: Yuan, Perry ; dri-devel@lists.freedesktop.org; linux- >> ker...@vger.kernel.org; Huang, Shimmer ; Huang, >> Ray >> Subject: Re: [PATCH v2] drm/dp: Fix aux->transfer NULL pointer dereference on >> drm_dp_dpcd_access >> >> [CAUTION: External Email] >> >> On Mon, 01 Nov 2021, Perry Yuan wrote: >> > Fix below crash by adding a check in the drm_dp_dpcd_access which >> > ensures that aux->transfer was actually initialized earlier. >> >> Gut feeling says this is papering over a real usage issue somewhere else. >> Why is >> the aux being used for transfers before ->transfer has been set? Why should >> the >> dp helper be defensive against all kinds of misprogramming? >> >> >> BR, >> Jani. >> > > The issue was found by Intel IGT test suite, graphic by pass test case. > https://gitlab.freedesktop.org/drm/igt-gpu-tools > normally use case will not see the issue. > To avoid this issue happy again when we run the test case , it will be nice > to add a check before the transfer is called. > And we can see that it really needs to have a check here to make ITG &kernel > happy. You're missing my point. What is the root cause? Why do you have the aux device or connector registered before ->transfer function is initialized. I don't think you should do that. BR, Jani. > > Perry. > >> >> > >> > BUG: kernel NULL pointer dereference, address: PGD 0 >> > P4D 0 >> > Oops: 0010 [#1] SMP NOPTI >> > RIP: 0010:0x0 >> > Code: Unable to access opcode bytes at RIP 0xffd6. >> > RSP: 0018:a8d64225bab8 EFLAGS: 00010246 >> > RAX: RBX: 0020 RCX: a8d64225bb5e >> > RDX: 93151d921880 RSI: a8d64225bac8 RDI: 931511a1a9d8 >> > RBP: a8d64225bb10 R08: 0001 R09: a8d64225ba60 >> > R10: 0002 R11: 000d R12: 0001 >> > R13: R14: a8d64225bb5e R15: 931511a1a9d8 >> > FS: 7ff8ea7fa9c0() GS:9317fe6c() >> > knlGS: >> > CS: 0010 DS: ES: CR0: 80050033 >> > CR2: ffd6 CR3: 00010d5a4000 CR4: 00750ee0 >> > PKRU: 5554 >> > Call Trace: >> > drm_dp_dpcd_access+0x72/0x110 [drm_kms_helper] >> > drm_dp_dpcd_read+0xb7/0xf0 [drm_kms_helper] >> > drm_dp_start_crc+0x38/0xb0 [drm_kms_helper] >> > amdgpu_dm_crtc_set_crc_source+0x1ae/0x3e0 [amdgpu] >> > crtc_crc_open+0x174/0x220 [drm] >> > full_proxy_open+0x168/0x1f0 >> > ? open_proxy_open+0x100/0x100 >> > do_dentry_open+0x156/0x370 >> > vfs_open+0x2d/0x30 >> > >> > v2: fix some typo >> > >> > Signed-off-by: Perry Yuan >> > --- >> > drivers/gpu/drm/drm_dp_helper.c | 4 >> > 1 file changed, 4 insertions(+) >> > >> > diff --git a/drivers/gpu/drm/drm_dp_helper.c >> > b/drivers/gpu/drm/drm_dp_helper.c index 6d0f2c447f3b..76b28396001a >> > 100644 >> > --- a/drivers/gpu/drm/drm_dp_helper.c >> > +++ b/drivers/gpu/drm/drm_dp_helper.c >> > @@ -260,6 +260,10 @@ static int drm_dp_dpcd_access(struct drm_dp_aux >> *aux, u8 request, >> > msg.buffer = buffer; >> > msg.size = size; >> > >> > + /* No transfer function is set, so not an available DP connector */ >> > + if (!aux->transfer) >> > + return -EINVAL; >> > + >> > mutex_lock(&aux->hw_mutex); >> > >> > /* >> >> -- >> Jani Nikula, Intel Open Source Graphics Center -- Jani Nikula, Intel Open Source Graphics Center
[PATCH -V2] drm/sun4i: Grab reference of connector before return connector from sun4i_tcon_get_connector
>From the comments of drm_for_each_connector_iter(), we know
that "connector is only valid within the list body, if you
want to use connector after calling drm_connector_list_iter_end()
then you need to grab your own reference first using
drm_connector_get()". So fix the wrong use of connector
according to the comments and then call drm_connector_put()
after using connector finishes.
Signed-off-by: He Ying
---
V2:
Use proper subject prefix
drivers/gpu/drm/sun4i/sun4i_tcon.c | 18 +-
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 9f06dec0fc61..24fa6784ee5f 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -47,12 +47,12 @@ static struct drm_connector *sun4i_tcon_get_connector(const
struct drm_encoder *
drm_connector_list_iter_begin(encoder->dev, &iter);
drm_for_each_connector_iter(connector, &iter)
if (connector->encoder == encoder) {
- drm_connector_list_iter_end(&iter);
- return connector;
+ drm_connector_get(connector);
+ break;
}
drm_connector_list_iter_end(&iter);
- return NULL;
+ return connector;
}
static int sun4i_tcon_get_pixel_depth(const struct drm_encoder *encoder)
@@ -65,6 +65,7 @@ static int sun4i_tcon_get_pixel_depth(const struct
drm_encoder *encoder)
return -EINVAL;
info = &connector->display_info;
+ drm_connector_put(connector);
if (info->num_bus_formats != 1)
return -EINVAL;
@@ -361,6 +362,7 @@ static void sun4i_tcon0_mode_set_cpu(struct sun4i_tcon
*tcon,
/* TODO support normal CPU interface modes */
struct sun6i_dsi *dsi = encoder_to_sun6i_dsi(encoder);
struct mipi_dsi_device *device = dsi->device;
+ struct drm_connector *connector;
u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
u8 lanes = device->lanes;
u32 block_space, start_delay;
@@ -372,7 +374,9 @@ static void sun4i_tcon0_mode_set_cpu(struct sun4i_tcon
*tcon,
sun4i_tcon0_mode_set_common(tcon, mode);
/* Set dithering if needed */
- sun4i_tcon0_mode_set_dithering(tcon, sun4i_tcon_get_connector(encoder));
+ connector = sun4i_tcon_get_connector(encoder);
+ sun4i_tcon0_mode_set_dithering(tcon, connector);
+ drm_connector_put(connector);
regmap_update_bits(tcon->regs, SUN4I_TCON0_CTL_REG,
SUN4I_TCON0_CTL_IF_MASK,
@@ -430,6 +434,7 @@ static void sun4i_tcon0_mode_set_lvds(struct sun4i_tcon
*tcon,
const struct drm_display_mode *mode)
{
unsigned int bp;
+ struct drm_connector *connector;
u8 clk_delay;
u32 reg, val = 0;
@@ -440,7 +445,9 @@ static void sun4i_tcon0_mode_set_lvds(struct sun4i_tcon
*tcon,
sun4i_tcon0_mode_set_common(tcon, mode);
/* Set dithering if needed */
- sun4i_tcon0_mode_set_dithering(tcon, sun4i_tcon_get_connector(encoder));
+ connector = sun4i_tcon_get_connector(encoder);
+ sun4i_tcon0_mode_set_dithering(tcon, connector);
+ drm_connector_put(connector);
/* Adjust clock delay */
clk_delay = sun4i_tcon_get_clk_delay(mode, 0);
@@ -518,6 +525,7 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon
*tcon,
/* Set dithering if needed */
sun4i_tcon0_mode_set_dithering(tcon, connector);
+ drm_connector_put(connector);
/* Adjust clock delay */
clk_delay = sun4i_tcon_get_clk_delay(mode, 0);
--
2.17.1
Re: [PATCH v2 1/2] drm/i915/ttm: Reorganize the ttm move code
On 01/11/2021 18:38, Thomas Hellström wrote: We are about to introduce failsafe- and asynchronous migration and ttm moves. This will add complexity and code to the TTM move code so it makes sense to split it out to a separate file to make the i915 TTM code easer to digest. Split the i915 TTM move code out and since we will have to change the name of the gpu_binds_iomem() and cpu_maps_iomem() functions anyway, we alter the name of gpu_binds_iomem() to i915_ttm_gtt_binds_lmem() which is more reflecting what it is used for. With this we also add some more documentation. Otherwise there should be no functional change. Signed-off-by: Thomas Hellström Reviewed-by: Matthew Auld
Re: [PATCH] drm/amd/display: remove unnecessary conditional operators
Reviewed-by: Simon Ser
Re: [PATCH v2 1/2] drm/aperture: Move conflicting fbdev frame buffer removal to a helper
Am 25.10.21 um 09:57 schrieb Javier Martinez Canillas:
The logic to remove the conflicting frame buffers for fbdev devices that
use a given memory range is only compiled if CONFIG_FB option is enabled.
But having an ifdefery in drm_aperture_remove_conflicting_framebuffers()
makes the function harder to extend. Move the logic into its own helper.
Suggested-by: Thomas Zimmermann
Signed-off-by: Javier Martinez Canillas
Acked-by: Thomas Zimmermann
---
(no changes since v1)
drivers/gpu/drm/drm_aperture.c | 39 ++
1 file changed, 26 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/drm_aperture.c b/drivers/gpu/drm/drm_aperture.c
index 74bd4a76b253..1a8ed0c616d6 100644
--- a/drivers/gpu/drm/drm_aperture.c
+++ b/drivers/gpu/drm/drm_aperture.c
@@ -273,6 +273,30 @@ static void drm_aperture_detach_drivers(resource_size_t
base, resource_size_t si
mutex_unlock(&drm_apertures_lock);
}
+static int drm_aperture_remove_conflicting_fbdev_framebuffers(resource_size_t base,
+ resource_size_t
size, bool primary,
+ const struct
drm_driver *req_driver)
+{
+#if IS_REACHABLE(CONFIG_FB)
+ struct apertures_struct *a;
+ int ret;
+
+ a = alloc_apertures(1);
+ if (!a)
+ return -ENOMEM;
+
+ a->ranges[0].base = base;
+ a->ranges[0].size = size;
+
+ ret = remove_conflicting_framebuffers(a, req_driver->name, primary);
+ kfree(a);
+
+ if (ret)
+ return ret;
+#endif
+ return 0;
+}
+
/**
* drm_aperture_remove_conflicting_framebuffers - remove existing
framebuffers in the given range
* @base: the aperture's base address in physical memory
@@ -289,23 +313,12 @@ static void drm_aperture_detach_drivers(resource_size_t
base, resource_size_t si
int drm_aperture_remove_conflicting_framebuffers(resource_size_t base,
resource_size_t size,
bool primary, const struct
drm_driver *req_driver)
{
-#if IS_REACHABLE(CONFIG_FB)
- struct apertures_struct *a;
int ret;
- a = alloc_apertures(1);
- if (!a)
- return -ENOMEM;
-
- a->ranges[0].base = base;
- a->ranges[0].size = size;
-
- ret = remove_conflicting_framebuffers(a, req_driver->name, primary);
- kfree(a);
-
+ ret = drm_aperture_remove_conflicting_fbdev_framebuffers(base, size,
primary,
+req_driver);
if (ret)
return ret;
-#endif
drm_aperture_detach_drivers(base, size);
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
[PATCH v2 1/3] drm/bridge: parade-ps8640: Don't try to enable VDO if poweron fails
If the bridge cannot get powered on, there's no reason to try to
communicate with it: change the ps8640_bridge_poweron function to
return an error value to the caller, so that we can avoid calling
ps8640_bridge_vdo_control() in ps8640_pre_enable() if the poweron
sequence fails.
Signed-off-by: AngeloGioacchino Del Regno
---
drivers/gpu/drm/bridge/parade-ps8640.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c
b/drivers/gpu/drm/bridge/parade-ps8640.c
index 4b36e4dc78f1..8c5402947b3c 100644
--- a/drivers/gpu/drm/bridge/parade-ps8640.c
+++ b/drivers/gpu/drm/bridge/parade-ps8640.c
@@ -293,19 +293,19 @@ static int ps8640_bridge_vdo_control(struct ps8640
*ps_bridge,
return 0;
}
-static void ps8640_bridge_poweron(struct ps8640 *ps_bridge)
+static int ps8640_bridge_poweron(struct ps8640 *ps_bridge)
{
struct regmap *map = ps_bridge->regmap[PAGE2_TOP_CNTL];
int ret, status;
if (ps_bridge->powered)
- return;
+ return 0;
ret = regulator_bulk_enable(ARRAY_SIZE(ps_bridge->supplies),
ps_bridge->supplies);
if (ret < 0) {
DRM_ERROR("cannot enable regulators %d\n", ret);
- return;
+ return ret;
}
gpiod_set_value(ps_bridge->gpio_powerdown, 0);
@@ -352,11 +352,13 @@ static void ps8640_bridge_poweron(struct ps8640
*ps_bridge)
ps_bridge->powered = true;
- return;
+ return 0;
err_regulators_disable:
regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
ps_bridge->supplies);
+
+ return ret;
}
static void ps8640_bridge_poweroff(struct ps8640 *ps_bridge)
@@ -381,7 +383,9 @@ static void ps8640_pre_enable(struct drm_bridge *bridge)
struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
int ret;
- ps8640_bridge_poweron(ps_bridge);
+ ret = ps8640_bridge_poweron(ps_bridge);
+ if (ret)
+ return;
ret = ps8640_bridge_vdo_control(ps_bridge, ENABLE);
if (ret < 0)
--
2.33.1
[PATCH v2 2/3] drm/bridge: parade-ps8640: Move real poweroff action to new function
In preparation for varying the poweron error handling in function
ps8640_bridge_poweron(), move function ps8640_bridge_poweroff() up
and also move the actual logic to power off the chip to a new
__ps8640_bridge_poweroff() function.
Signed-off-by: AngeloGioacchino Del Regno
---
drivers/gpu/drm/bridge/parade-ps8640.c | 37 ++
1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c
b/drivers/gpu/drm/bridge/parade-ps8640.c
index 8c5402947b3c..41f5d511d516 100644
--- a/drivers/gpu/drm/bridge/parade-ps8640.c
+++ b/drivers/gpu/drm/bridge/parade-ps8640.c
@@ -293,6 +293,26 @@ static int ps8640_bridge_vdo_control(struct ps8640
*ps_bridge,
return 0;
}
+static void __ps8640_bridge_poweroff(struct ps8640 *ps_bridge)
+{
+ gpiod_set_value(ps_bridge->gpio_reset, 1);
+ gpiod_set_value(ps_bridge->gpio_powerdown, 1);
+ if (regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
+ ps_bridge->supplies)) {
+ DRM_ERROR("cannot disable regulators\n");
+ }
+}
+
+static void ps8640_bridge_poweroff(struct ps8640 *ps_bridge)
+{
+ if (!ps_bridge->powered)
+ return;
+
+ __ps8640_bridge_poweroff(ps_bridge);
+
+ ps_bridge->powered = false;
+}
+
static int ps8640_bridge_poweron(struct ps8640 *ps_bridge)
{
struct regmap *map = ps_bridge->regmap[PAGE2_TOP_CNTL];
@@ -361,23 +381,6 @@ static int ps8640_bridge_poweron(struct ps8640 *ps_bridge)
return ret;
}
-static void ps8640_bridge_poweroff(struct ps8640 *ps_bridge)
-{
- int ret;
-
- if (!ps_bridge->powered)
- return;
-
- gpiod_set_value(ps_bridge->gpio_reset, 1);
- gpiod_set_value(ps_bridge->gpio_powerdown, 1);
- ret = regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
-ps_bridge->supplies);
- if (ret < 0)
- DRM_ERROR("cannot disable regulators %d\n", ret);
-
- ps_bridge->powered = false;
-}
-
static void ps8640_pre_enable(struct drm_bridge *bridge)
{
struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
--
2.33.1
[PATCH v2 3/3] drm/bridge: parade-ps8640: Perform full poweroff if poweron fails
In function ps8640_bridge_poweron(), in case of a failure not relative
to the regulators enablement, the code was disabling the regulators but
the gpio changes that happened during the poweron sequence were not
being reverted back to a clean poweroff state.
Since it is expected that, when we enter ps8640_bridge_poweron(), the
powerdown and reset GPIOs are both in active state exactly as they were
left in the poweroff function before, we can simply call function
__ps8640_bridge_poweroff() in the failure case, reverting every change
that was done during the power on sequence.
Of course it was chosen to call the poweroff function instead of adding
code to revert the GPIO changes to the poweron one to avoid duplicating
code, as we would be doing exactly what the poweroff function does.
Signed-off-by: AngeloGioacchino Del Regno
---
drivers/gpu/drm/bridge/parade-ps8640.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c
b/drivers/gpu/drm/bridge/parade-ps8640.c
index 41f5d511d516..ef1b51d8b676 100644
--- a/drivers/gpu/drm/bridge/parade-ps8640.c
+++ b/drivers/gpu/drm/bridge/parade-ps8640.c
@@ -344,7 +344,7 @@ static int ps8640_bridge_poweron(struct ps8640 *ps_bridge)
if (ret < 0) {
DRM_ERROR("failed read PAGE2_GPIO_H: %d\n", ret);
- goto err_regulators_disable;
+ goto err_poweroff;
}
msleep(50);
@@ -360,23 +360,22 @@ static int ps8640_bridge_poweron(struct ps8640 *ps_bridge)
ret = regmap_update_bits(map, PAGE2_MCS_EN, MCS_EN, 0);
if (ret < 0) {
DRM_ERROR("failed write PAGE2_MCS_EN: %d\n", ret);
- goto err_regulators_disable;
+ goto err_poweroff;
}
/* Switch access edp panel's edid through i2c */
ret = regmap_write(map, PAGE2_I2C_BYPASS, I2C_BYPASS_EN);
if (ret < 0) {
DRM_ERROR("failed write PAGE2_I2C_BYPASS: %d\n", ret);
- goto err_regulators_disable;
+ goto err_poweroff;
}
ps_bridge->powered = true;
return 0;
-err_regulators_disable:
- regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
- ps_bridge->supplies);
+err_poweroff:
+ __ps8640_bridge_poweroff(ps_bridge);
return ret;
}
--
2.33.1
[PATCH v4 4/5] drm/msm/dp: Enable downspread for supported DP sinks
The sc7280 eDP sink that supports downspread will fail link training
if source does not enable SSC / downspread. This change will set the
downspread bit in the DP sink if supported and indicate SSC support
to the DP PHY driver.
Signed-off-by: Sankeerth Billakanti
Reviewed-by: Stephen Boyd
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 62e75dc..a40e798 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1228,7 +1228,9 @@ static int dp_ctrl_link_train(struct dp_ctrl_private
*ctrl,
int *training_step)
{
int ret = 0;
+ const u8 *dpcd = ctrl->panel->dpcd;
u8 encoding = DP_SET_ANSI_8B10B;
+ u8 ssc;
struct dp_link_info link_info = {0};
dp_ctrl_config_ctrl(ctrl);
@@ -1238,6 +1240,12 @@ static int dp_ctrl_link_train(struct dp_ctrl_private
*ctrl,
link_info.capabilities = DP_LINK_CAP_ENHANCED_FRAMING;
dp_aux_link_configure(ctrl->aux, &link_info);
+
+ if (drm_dp_max_downspread(dpcd)) {
+ ssc = DP_SPREAD_AMP_0_5;
+ drm_dp_dpcd_write(ctrl->aux, DP_DOWNSPREAD_CTRL, &ssc, 1);
+ }
+
drm_dp_dpcd_write(ctrl->aux, DP_MAIN_LINK_CHANNEL_CODING_SET,
&encoding, 1);
@@ -1312,9 +1320,11 @@ static int dp_ctrl_enable_mainlink_clocks(struct
dp_ctrl_private *ctrl)
struct dp_io *dp_io = &ctrl->parser->io;
struct phy *phy = dp_io->phy;
struct phy_configure_opts_dp *opts_dp = &dp_io->phy_opts.dp;
+ const u8 *dpcd = ctrl->panel->dpcd;
opts_dp->lanes = ctrl->link->link_params.num_lanes;
opts_dp->link_rate = ctrl->link->link_params.rate / 100;
+ opts_dp->ssc = drm_dp_max_downspread(dpcd);
dp_ctrl_set_clock_rate(ctrl, DP_CTRL_PM, "ctrl_link",
ctrl->link->link_params.rate * 1000);
@@ -1406,7 +1416,7 @@ void dp_ctrl_host_deinit(struct dp_ctrl *dp_ctrl)
static bool dp_ctrl_use_fixed_nvid(struct dp_ctrl_private *ctrl)
{
- u8 *dpcd = ctrl->panel->dpcd;
+ const u8 *dpcd = ctrl->panel->dpcd;
/*
* For better interop experience, used a fixed NVID=0x8000
--
2.7.4
[PATCH v4 1/5] dt-bindings: msm/dp: Add DP compatible strings for sc7280
From: Sankeerth Billakanti The Qualcomm SC7280 platform supports one eDP controller and a DP controller. This change will add the compatible string for both eDP and DP to msm dp-controller binding. Signed-off-by: Sankeerth Billakanti changes in v3: - Modify the subject (Doug Anderson) - Add sc7280-dp also to the list (Stephen Boyd) changes in v2: - Sort alphabetically (Stephen Boyd) - Cleanup residual stale changes in the patch (Matthias Kaehlcke) - Modify the subject (Doug Anderson) Reviewed-by: Stephen Boyd --- Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml index 63e585f..5457612 100644 --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml @@ -17,6 +17,8 @@ properties: compatible: enum: - qcom,sc7180-dp + - qcom,sc7280-dp + - qcom,sc7280-edp - qcom,sc8180x-dp - qcom,sc8180x-edp -- 2.7.4
[PATCH v4 0/5] Add support for eDP controller on SC7280
This series will add eDP controller support for Qualcomm SC7280 platform. These patches are baseline changes with which we can enable eDP display on sc7280. The sc7280 eDP controller driver can also support additional features such as no_hpd detection, PSR, etc. which will be enabled in subsequent patch series. This is based on Bjorn's changes in the below mentioned series to support both eDP and DP programming through the same driver: https://patchwork.kernel.org/project/linux-arm-msm/list/?series=564841 Sankeerth Billakanti (5): dt-bindings: msm/dp: Add DP compatible strings for sc7280 drm/msm/dp: Add DP controllers for sc7280 drm/dp: Add macro to check max_downspread capability drm/msm/dp: Enable downspread for supported DP sinks drm/msm/dp: Enable ASSR for supported DP sinks .../bindings/display/msm/dp-controller.yaml| 2 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++-- drivers/gpu/drm/msm/dp/dp_ctrl.c | 23 +++--- drivers/gpu/drm/msm/dp/dp_display.c| 9 + include/drm/drm_dp_helper.h| 7 +++ 5 files changed, 40 insertions(+), 5 deletions(-) -- 2.7.4
[PATCH v4 2/5] drm/msm/dp: Add DP controllers for sc7280
The eDP controller on SC7280 is similar to the eDP/DP controllers
supported by the current driver implementation.
SC7280 supports one EDP and one DP controller which can operate
concurrently.
This change adds the support for eDP and DP controller on sc7280.
Signed-off-by: Sankeerth Billakanti
changes in v3:
- Split into patches according to function (Dmitry Baryshkov)
- Use DP_CONTROLLER_1 for eDP controller intf (Dmitry Baryshkov)
- Use DP_CONTROLLER_0 for sc7280-dp (Dmitry Baryshkov)
- Add macro in drm_helper.h for checking ssc capability (Stephen Boyd)
- Use existing macro to check assr capability (Stephen Boyd)
- Add comment for HPD_INIT_SETUP delay (Stephen Boyd)
changes in v2:
- Don't initialize variables to 0 (Stephen Boyd)
- Use const for read-only dpcd (Stephen Boyd)
- Remove zero pixel clock check (Stephen Boyd)
- Sort compatible strings alphabetically (Stephen Boyd)
- Use pwm_bl.c for backlight instead of gpio (Stephen Boyd)
- Change return type for functions returning always 0 (Matthias Kaehlcke)
Reviewed-by: Stephen Boyd
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
drivers/gpu/drm/msm/dp/dp_display.c| 9 +
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index ce6f32a..2b4dc9d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -856,9 +856,9 @@ static const struct dpu_intf_cfg sm8150_intf[] = {
};
static const struct dpu_intf_cfg sc7280_intf[] = {
- INTF_BLK("intf_0", INTF_0, 0x34000, INTF_DP, 0, 24, INTF_SC7280_MASK,
MDP_SSPP_TOP0_INTR, 24, 25),
+ INTF_BLK("intf_0", INTF_0, 0x34000, INTF_DP, MSM_DP_CONTROLLER_0, 24,
INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
INTF_BLK("intf_1", INTF_1, 0x35000, INTF_DSI, 0, 24, INTF_SC7280_MASK,
MDP_SSPP_TOP0_INTR, 26, 27),
- INTF_BLK("intf_5", INTF_5, 0x39000, INTF_EDP, 0, 24, INTF_SC7280_MASK,
MDP_SSPP_TOP0_INTR, 22, 23),
+ INTF_BLK("intf_5", INTF_5, 0x39000, INTF_DP, MSM_DP_CONTROLLER_1, 24,
INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 22, 23),
};
/*
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index aba8aa4..61385d6 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -135,8 +135,17 @@ static const struct msm_dp_config sc7180_dp_cfg = {
.num_descs = 1,
};
+static const struct msm_dp_config sc7280_dp_cfg = {
+ .descs = (const struct msm_dp_desc[]) {
+ [MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae9,
.connector_type = DRM_MODE_CONNECTOR_DisplayPort },
+ [MSM_DP_CONTROLLER_1] = { .io_start = 0x0aea,
.connector_type = DRM_MODE_CONNECTOR_eDP },
+ },
+ .num_descs = 2,
+};
+
static const struct of_device_id dp_dt_match[] = {
{ .compatible = "qcom,sc7180-dp", .data = &sc7180_dp_cfg },
+ { .compatible = "qcom,sc7280-edp", .data = &sc7280_dp_cfg },
{}
};
--
2.7.4
[PATCH v4 3/5] drm/dp: Add macro to check max_downspread capability
Add a macro to check for the max_downspread capability in
drm_dp_helper.
Signed-off-by: Sankeerth Billakanti
Reviewed-by: Stephen Boyd
changes in v4:
- Return 1 for DPCD version >= v1.1 (Stephen Boyd)
---
include/drm/drm_dp_helper.h | 7 +++
1 file changed, 7 insertions(+)
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index b52df4d..596635c 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1767,6 +1767,13 @@ drm_dp_tps3_supported(const u8
dpcd[DP_RECEIVER_CAP_SIZE])
}
static inline bool
+drm_dp_max_downspread(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
+{
+ return dpcd[DP_DPCD_REV] >= 0x11 ||
+ dpcd[DP_MAX_DOWNSPREAD] & DP_MAX_DOWNSPREAD_0_5;
+}
+
+static inline bool
drm_dp_tps4_supported(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
{
return dpcd[DP_DPCD_REV] >= 0x14 &&
--
2.7.4
[PATCH v4 5/5] drm/msm/dp: Enable ASSR for supported DP sinks
The eDP sink on sc7280 supports ASSR and dp driver will
enable ASSR in the source hardware. The driver needs to
enable the ASSR field in the DPCD configuration register
to avoid screen corruption. This change will enable ASSR
if supported in the sink device.
Signed-off-by: Sankeerth Billakanti
Reviewed-by: Stephen Boyd
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index a40e798..c724cb0 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -119,13 +119,13 @@ void dp_ctrl_push_idle(struct dp_ctrl *dp_ctrl)
static void dp_ctrl_config_ctrl(struct dp_ctrl_private *ctrl)
{
u32 config = 0, tbd;
- u8 *dpcd = ctrl->panel->dpcd;
+ const u8 *dpcd = ctrl->panel->dpcd;
/* Default-> LSCLK DIV: 1/4 LCLK */
config |= (2 << DP_CONFIGURATION_CTRL_LSCLK_DIV_SHIFT);
/* Scrambler reset enable */
- if (dpcd[DP_EDP_CONFIGURATION_CAP] & DP_ALTERNATE_SCRAMBLER_RESET_CAP)
+ if (drm_dp_alternate_scrambler_reset_cap(dpcd))
config |= DP_CONFIGURATION_CTRL_ASSR;
tbd = dp_link_get_test_bits_depth(ctrl->link,
@@ -1231,6 +1231,7 @@ static int dp_ctrl_link_train(struct dp_ctrl_private
*ctrl,
const u8 *dpcd = ctrl->panel->dpcd;
u8 encoding = DP_SET_ANSI_8B10B;
u8 ssc;
+ u8 assr;
struct dp_link_info link_info = {0};
dp_ctrl_config_ctrl(ctrl);
@@ -1249,6 +1250,12 @@ static int dp_ctrl_link_train(struct dp_ctrl_private
*ctrl,
drm_dp_dpcd_write(ctrl->aux, DP_MAIN_LINK_CHANNEL_CODING_SET,
&encoding, 1);
+ if (drm_dp_alternate_scrambler_reset_cap(dpcd)) {
+ assr = DP_ALTERNATE_SCRAMBLER_RESET_ENABLE;
+ drm_dp_dpcd_write(ctrl->aux, DP_EDP_CONFIGURATION_SET,
+ &assr, 1);
+ }
+
ret = dp_ctrl_link_train_1(ctrl, training_step);
if (ret) {
DRM_ERROR("link training #1 failed. ret=%d\n", ret);
--
2.7.4
Re: [Intel-gfx] [PATCH v3 05/10] drm/i915: Prepare for multiple gts
On 01/11/2021 23:11, Andi Shyti wrote:
Hi Matt and Tvrtko,
[...]
static int
intel_gt_tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t
phys_addr)
we don't actually need 'id', it's gt->info.id. It's introduced in
patch 3 with the value '0' but it's not needed.
I have a suspicion code got munged up over endless rebases and refactors.
This patch is the one which introduces the id member to gt->info. But it is not
setting it, even though I suspect the intent was for intel_gt_tile_setup to do
that.
Instead gt->info.id is only set to a valid value in last patch of this series
inside intel_gt_probe_all:
+ gt->i915 = i915;
+ gt->name = gtdef->name;
+ gt->type = gtdef->type;
+ gt->info.engine_mask = gtdef->engine_mask;
+ gt->info.id = i;
+
+ drm_dbg(&i915->drm, "Setting up %s %u\n", gt->name,
gt->info.id);
+ ret = intel_gt_tile_setup(gt, i, phys_addr +
gtdef->mapping_base);
And intel_gt_tile_setup then calls __intel_gt_init_early which assigns gt->i915
yet again.
So I'd say there is probably space to bring this all into a more streamlined
flow, even more than what you suggest below.
Regards,
Tvrtko
{
+ struct drm_i915_private *i915 = gt->i915;
+ struct intel_uncore *uncore;
+ struct intel_uncore_mmio_debug *mmio_debug;
int ret;
- intel_uncore_init_early(gt->uncore, gt);
+ if (id) {
if (gt->info.id) ?
Andi
+ uncore = kzalloc(sizeof(*uncore), GFP_KERNEL);
+ if (!uncore)
+ return -ENOMEM;
+
+ mmio_debug = kzalloc(sizeof(*mmio_debug), GFP_KERNEL);
+ if (!mmio_debug) {
+ kfree(uncore);
+ return -ENOMEM;
+ }
+
+ __intel_gt_init_early(gt, uncore, i915);
+ } else {
+ uncore = &i915->uncore;
+ mmio_debug = &i915->mmio_debug;
+ }
+
+ intel_uncore_init_early(uncore, gt);
ret = intel_uncore_setup_mmio(gt->uncore, phys_addr);
Re: amdgpu on Ryzen 5600G -- 'purple' background [WAS: Re: amdgpu "Fatal error during GPU init"; Ryzen 5600G integrated GPU + kernel 5.14.13}
On 10/30/21 11:24, PGNet Dev wrote: Is that kernel/modconfig config incorrect? Not relevant to the problem? Something else(where) needed? fwiw, AMD Global Customer Care's response to question about this 'purple' issue: "... Please be informed that Ryzen 5600G APU is supported only on Windows 10, 11, RHEL and Ubuntu OS. Also, this might vary depending on the manufacturer in your case AsRock. https://www.amd.com/en/products/apu/amd-ryzen-5-5600g I suggest you try installing Ubuntu 20.04.3 and update to latest version and check the status. "
[PATCH] backlight: led_bl: Add support for an "enable" GPIO
From: Jean-Jacques Hiblot
This patch adds support for an "enable GPIO".
Signed-off-by: Jean-Jacques Hiblot
Signed-off-by: Corentin LABBE
---
drivers/video/backlight/led_bl.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
index f54d256e2d54..ebd7acc32980 100644
--- a/drivers/video/backlight/led_bl.c
+++ b/drivers/video/backlight/led_bl.c
@@ -8,6 +8,7 @@
#include
#include
+#include
#include
#include
@@ -15,6 +16,7 @@ struct led_bl_data {
struct device *dev;
struct backlight_device *bl_dev;
struct led_classdev **leds;
+ struct gpio_desc*enable_gpio;
boolenabled;
int nb_leds;
unsigned int*levels;
@@ -35,6 +37,9 @@ static void led_bl_set_brightness(struct led_bl_data *priv,
int level)
for (i = 0; i < priv->nb_leds; i++)
led_set_brightness(priv->leds[i], bkl_brightness);
+ if (!priv->enabled)
+ gpiod_set_value_cansleep(priv->enable_gpio, 1);
+
priv->enabled = true;
}
@@ -48,6 +53,9 @@ static void led_bl_power_off(struct led_bl_data *priv)
for (i = 0; i < priv->nb_leds; i++)
led_set_brightness(priv->leds[i], LED_OFF);
+ if (priv->enabled)
+ gpiod_set_value_cansleep(priv->enable_gpio, 0);
+
priv->enabled = false;
}
@@ -209,6 +217,11 @@ static int led_bl_probe(struct platform_device *pdev)
return PTR_ERR(priv->bl_dev);
}
+ priv->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(priv->enable_gpio))
+ return PTR_ERR(priv->enable_gpio);
+
for (i = 0; i < priv->nb_leds; i++)
led_sysfs_disable(priv->leds[i]);
--
2.25.1
[Bug 214901] amdgpu freezes HP laptop at start up
https://bugzilla.kernel.org/show_bug.cgi?id=214901 --- Comment #5 from spassw...@web.de --- This commit leads to a freeze when starting https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/gpu/drm/amd/amdgpu?id=714d9e4574d54596973ee3b0624ee4a16264d700 After reverting it the kernel 5.15 boots normally -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[PATCH v5 0/3] drm/i915: Prepare error capture for asynchronous migration
This patch series prepares error capture for asynchronous migration, where the vma pages may not reflect the pages the GPU is currently executing from but may be several migrations ahead. The first patch introduces vma state snapshots that record the vma state at request submission time. It also takes additional measures to make sure that the capture list and request is not disappearing from under us while capturing. The latter may otherwise happen if a heartbeat triggered parallel capture is running during a manual reset which retires the request. The second patch changes the allocation mode during capture to reflect that capturing is typically done in the fence signalling critical path. More details on the patch itself. Finally the last patch is more of a POC patch and not strictly needed yet, but will be (or at least something very similar) soon for async unbinding. It will make sure that unbinding doesn't complete or signal completion before capture is done. Async reuse of memory can't happen until unbinding signals complete and without waiting for capture done, we might capture contents of reused memory. Before the last patch the vma active is instead still keeping the vma alive, but that will not work with async unbinding anymore, and also it is still not clear how we guarantee keeping the vma alive long enough to even grab an active reference during capture. v2: - Mostly Fixes for selftests and rebinding. See patch 3. v3: - Honor the unbind fence also when evicting for suspend on gen6. - Minor cleanups on patch 3. v4: - Break out patch 2 from patch 1. v5: - Ditch a patch from the since it's already commited. - Use __GFP_KSWAPD_RECLAIM rather than GFP_NOWAIT in patch 2. Thomas Hellström (3): drm/i915: Update error capture code to avoid using the current vma state drm/i915: Use __GFP_KSWAPD_RECLAIM in the capture code drm/i915: Initial introduction of vma resources drivers/gpu/drm/i915/Makefile | 1 + .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 137 ++-- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 8 +- .../drm/i915/gt/intel_execlists_submission.c | 2 +- drivers/gpu/drm/i915/i915_gpu_error.c | 180 ++- drivers/gpu/drm/i915/i915_request.c | 63 -- drivers/gpu/drm/i915/i915_request.h | 20 +- drivers/gpu/drm/i915/i915_vma.c | 206 +- drivers/gpu/drm/i915/i915_vma.h | 20 +- drivers/gpu/drm/i915/i915_vma_snapshot.c | 131 +++ drivers/gpu/drm/i915/i915_vma_snapshot.h | 112 ++ drivers/gpu/drm/i915/i915_vma_types.h | 5 + drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 98 + 13 files changed, 837 insertions(+), 146 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_vma_snapshot.c create mode 100644 drivers/gpu/drm/i915/i915_vma_snapshot.h -- 2.31.1
[PATCH v5 1/3] drm/i915: Update error capture code to avoid using the current vma state
With asynchronous migrations, the vma state may be several migrations
ahead of the state that matches the request we're capturing.
Address that by introducing an i915_vma_snapshot structure that
can be used to snapshot relevant state at request submission.
In order to make sure we access the correct memory, the snapshots take
references on relevant sg-tables and memory regions.
Also move the capture list allocation out of the fence signaling
critical path and use the CONFIG_DRM_I915_CAPTURE_ERROR define to
avoid compiling in members and functions used for error capture
when they're not used.
Finally, Introducing lockdep annotation means we will be start seeing
lockdep splats in the capture code. This is because typically the
capture code runs in the fence signalling critical path. These splats
and the associated deadlocks will be worked around in an upcoming patch.
Splats look like these:
[ 234.842048] WARNING: possible circular locking dependency detected
[ 234.842050] 5.15.0-rc7+ #20 Tainted: G U W
[ 234.842052] --
[ 234.842054] gem_exec_captur/1180 is trying to acquire lock:
[ 234.842056] a3e51c00 (fs_reclaim){+.+.}-{0:0}, at:
__kmalloc+0x4d/0x330
[ 234.842063]
but task is already holding lock:
[ 234.842064] a3f57620 (dma_fence_map){}-{0:0}, at:
i915_vma_snapshot_resource_pin+0x27/0x30 [i915]
[ 234.842138]
which lock already depends on the new lock.
[ 234.842140]
the existing dependency chain (in reverse order) is:
[ 234.842142]
-> #2 (dma_fence_map){}-{0:0}:
[ 234.842145]__dma_fence_might_wait+0x41/0xa0
[ 234.842149]dma_resv_lockdep+0x1dc/0x28f
[ 234.842151]do_one_initcall+0x58/0x2d0
[ 234.842154]kernel_init_freeable+0x273/0x2bf
[ 234.842157]kernel_init+0x16/0x120
[ 234.842160]ret_from_fork+0x1f/0x30
[ 234.842163]
-> #1 (mmu_notifier_invalidate_range_start){+.+.}-{0:0}:
[ 234.842166]fs_reclaim_acquire+0x6d/0xd0
[ 234.842168]__kmalloc_node+0x51/0x3a0
[ 234.842171]alloc_cpumask_var_node+0x1b/0x30
[ 234.842174]native_smp_prepare_cpus+0xc7/0x292
[ 234.842177]kernel_init_freeable+0x160/0x2bf
[ 234.842179]kernel_init+0x16/0x120
[ 234.842181]ret_from_fork+0x1f/0x30
[ 234.842184]
-> #0 (fs_reclaim){+.+.}-{0:0}:
[ 234.842186]__lock_acquire+0x1161/0x1dc0
[ 234.842189]lock_acquire+0xb5/0x2b0
[ 234.842192]fs_reclaim_acquire+0xa1/0xd0
[ 234.842193]__kmalloc+0x4d/0x330
[ 234.842196]i915_vma_coredump_create+0x78/0x5b0 [i915]
[ 234.842253]intel_engine_coredump_add_vma+0x36/0xe0 [i915]
[ 234.842307]__i915_gpu_coredump+0x290/0x5e0 [i915]
[ 234.842365]i915_capture_error_state+0x57/0xa0 [i915]
[ 234.842415]intel_gt_handle_error+0x348/0x3e0 [i915]
[ 234.842462]intel_gt_debugfs_reset_store+0x3c/0x90 [i915]
[ 234.842504]simple_attr_write+0xc1/0xe0
[ 234.842507]full_proxy_write+0x53/0x80
[ 234.842509]vfs_write+0xbc/0x350
[ 234.842513]ksys_write+0x58/0xd0
[ 234.842514]do_syscall_64+0x38/0x90
[ 234.842516]entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 234.842519]
other info that might help us debug this:
[ 234.842521] Chain exists of:
fs_reclaim --> mmu_notifier_invalidate_range_start -->
dma_fence_map
[ 234.842526] Possible unsafe locking scenario:
[ 234.842528]CPU0CPU1
[ 234.842529]
[ 234.842531] lock(dma_fence_map);
[ 234.842532]
lock(mmu_notifier_invalidate_range_start);
[ 234.842535]lock(dma_fence_map);
[ 234.842537] lock(fs_reclaim);
[ 234.842539]
*** DEADLOCK ***
[ 234.842540] 4 locks held by gem_exec_captur/1180:
[ 234.842543] #0: 9007812d9460 (sb_writers#17){.+.+}-{0:0}, at:
ksys_write+0x58/0xd0
[ 234.842547] #1: 900781d9ecb8 (&attr->mutex){+.+.}-{3:3}, at:
simple_attr_write+0x3a/0xe0
[ 234.842552] #2: c11913a8 (capture_mutex){+.+.}-{3:3}, at:
i915_capture_error_state+0x1a/0xa0 [i915]
[ 234.842602] #3: a3f57620 (dma_fence_map){}-{0:0}, at:
i915_vma_snapshot_resource_pin+0x27/0x30 [i915]
[ 234.842656]
stack backtrace:
[ 234.842658] CPU: 0 PID: 1180 Comm: gem_exec_captur Tainted: G U W
5.15.0-rc7+ #20
[ 234.842661] Hardware name: ASUS System Product Name/PRIME B560M-A AC, BIOS
0403 01/26/2021
[ 234.842664] Call Trace:
[ 234.842666] dump_stack_lvl+0x57/0x72
[ 234.842669] check_noncircular+0xde/0x100
[ 234.842672] ? __lock_acquire+0x3bf/0x1dc0
[ 234.842675] __lock_acquire+0x1161/0x1dc0
[ 234.842678] lock_acquire+0xb5/0x2b0
[ 234.842680] ? __kmalloc+0x4d/0x330
[ 234.842683] ? finish_task_switch.isra.0+0xf2
[PATCH v5 2/3] drm/i915: Use __GFP_KSWAPD_RECLAIM in the capture code
The capture code is typically run entirely in the fence signalling
critical path. Recently added lockdep annotation reveals a lockdep splat
similar to the below one.
Fix the splats and the associated potential deadlocks using
__GFP_KSWAPD_RECLAIM (which is the same as GFP_WAIT, but open-coded for
clarity) rather than GFP_KERNEL for memory allocation in the capture path.
This has the potential drawback that capture might fail in situations with
memory pressure.
[ 234.842048] WARNING: possible circular locking dependency detected
[ 234.842050] 5.15.0-rc7+ #20 Tainted: G U W
[ 234.842052] --
[ 234.842054] gem_exec_captur/1180 is trying to acquire lock:
[ 234.842056] a3e51c00 (fs_reclaim){+.+.}-{0:0}, at:
__kmalloc+0x4d/0x330
[ 234.842063]
but task is already holding lock:
[ 234.842064] a3f57620 (dma_fence_map){}-{0:0}, at:
i915_vma_snapshot_resource_pin+0x27/0x30 [i915]
[ 234.842138]
which lock already depends on the new lock.
[ 234.842140]
the existing dependency chain (in reverse order) is:
[ 234.842142]
-> #2 (dma_fence_map){}-{0:0}:
[ 234.842145]__dma_fence_might_wait+0x41/0xa0
[ 234.842149]dma_resv_lockdep+0x1dc/0x28f
[ 234.842151]do_one_initcall+0x58/0x2d0
[ 234.842154]kernel_init_freeable+0x273/0x2bf
[ 234.842157]kernel_init+0x16/0x120
[ 234.842160]ret_from_fork+0x1f/0x30
[ 234.842163]
-> #1 (mmu_notifier_invalidate_range_start){+.+.}-{0:0}:
[ 234.842166]fs_reclaim_acquire+0x6d/0xd0
[ 234.842168]__kmalloc_node+0x51/0x3a0
[ 234.842171]alloc_cpumask_var_node+0x1b/0x30
[ 234.842174]native_smp_prepare_cpus+0xc7/0x292
[ 234.842177]kernel_init_freeable+0x160/0x2bf
[ 234.842179]kernel_init+0x16/0x120
[ 234.842181]ret_from_fork+0x1f/0x30
[ 234.842184]
-> #0 (fs_reclaim){+.+.}-{0:0}:
[ 234.842186]__lock_acquire+0x1161/0x1dc0
[ 234.842189]lock_acquire+0xb5/0x2b0
[ 234.842192]fs_reclaim_acquire+0xa1/0xd0
[ 234.842193]__kmalloc+0x4d/0x330
[ 234.842196]i915_vma_coredump_create+0x78/0x5b0 [i915]
[ 234.842253]intel_engine_coredump_add_vma+0x36/0xe0 [i915]
[ 234.842307]__i915_gpu_coredump+0x290/0x5e0 [i915]
[ 234.842365]i915_capture_error_state+0x57/0xa0 [i915]
[ 234.842415]intel_gt_handle_error+0x348/0x3e0 [i915]
[ 234.842462]intel_gt_debugfs_reset_store+0x3c/0x90 [i915]
[ 234.842504]simple_attr_write+0xc1/0xe0
[ 234.842507]full_proxy_write+0x53/0x80
[ 234.842509]vfs_write+0xbc/0x350
[ 234.842513]ksys_write+0x58/0xd0
[ 234.842514]do_syscall_64+0x38/0x90
[ 234.842516]entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 234.842519]
other info that might help us debug this:
[ 234.842521] Chain exists of:
fs_reclaim --> mmu_notifier_invalidate_range_start -->
dma_fence_map
[ 234.842526] Possible unsafe locking scenario:
[ 234.842528]CPU0CPU1
[ 234.842529]
[ 234.842531] lock(dma_fence_map);
[ 234.842532]
lock(mmu_notifier_invalidate_range_start);
[ 234.842535]lock(dma_fence_map);
[ 234.842537] lock(fs_reclaim);
[ 234.842539]
*** DEADLOCK ***
[ 234.842540] 4 locks held by gem_exec_captur/1180:
[ 234.842543] #0: 9007812d9460 (sb_writers#17){.+.+}-{0:0}, at:
ksys_write+0x58/0xd0
[ 234.842547] #1: 900781d9ecb8 (&attr->mutex){+.+.}-{3:3}, at:
simple_attr_write+0x3a/0xe0
[ 234.842552] #2: c11913a8 (capture_mutex){+.+.}-{3:3}, at:
i915_capture_error_state+0x1a/0xa0 [i915]
[ 234.842602] #3: a3f57620 (dma_fence_map){}-{0:0}, at:
i915_vma_snapshot_resource_pin+0x27/0x30 [i915]
[ 234.842656]
stack backtrace:
[ 234.842658] CPU: 0 PID: 1180 Comm: gem_exec_captur Tainted: G U W
5.15.0-rc7+ #20
[ 234.842661] Hardware name: ASUS System Product Name/PRIME B560M-A AC, BIOS
0403 01/26/2021
[ 234.842664] Call Trace:
[ 234.842666] dump_stack_lvl+0x57/0x72
[ 234.842669] check_noncircular+0xde/0x100
[ 234.842672] ? __lock_acquire+0x3bf/0x1dc0
[ 234.842675] __lock_acquire+0x1161/0x1dc0
[ 234.842678] lock_acquire+0xb5/0x2b0
[ 234.842680] ? __kmalloc+0x4d/0x330
[ 234.842683] ? finish_task_switch.isra.0+0xf2/0x360
[ 234.842686] ? i915_vma_coredump_create+0x78/0x5b0 [i915]
[ 234.842734] fs_reclaim_acquire+0xa1/0xd0
[ 234.842737] ? __kmalloc+0x4d/0x330
[ 234.842739] __kmalloc+0x4d/0x330
[ 234.842742] i915_vma_coredump_create+0x78/0x5b0 [i915]
[ 234.842793] ? capture_vma+0xbe/0x110 [i915]
[ 234.842844] intel_engine_coredump_add_vma+0x36/0xe0 [i915]
[ 234.842892] __i915_gpu_coredump+0x290/0x5e0 [i915]
[ 234.842939] i915_capt
[PATCH v5 3/3] drm/i915: Initial introduction of vma resources
The vma resource are needed for asynchronous bind management and are
similar to TTM resources. They contain the data needed for
asynchronous unbinding (typically the vm range, any backend
private information and a means to do refcounting and to hold
the unbinding for error capture).
When a vma is bound, a vma resource is created and attached to the
vma, and on async unbinding it is detached from the vma, and instead
the vm records the fence marking unbind complete. This fence needs to
be waited on before we can bind the same region again, so either
the fence can be recorded for this particular range only, using an
interval tree, or as a simpler approach, for the whole vm. The latter
means no binding can take place on a vm until all detached vma
resources scheduled for unbind are signaled. With an interval tree
fence recording, the interval tree needs to be searched for fences
to be signaled before binding can take place.
But most of that is for later, this patch only introduces stub vma
resources without unbind capability and the fences of which are waited
for sync during unbinding. At this point we're interested in the hold
capability as a POC for error capture. Note that the current sync waiting
at unbind time is done uninterruptible, but that's OK since we're
only ever waiting during error capture, and in that case there's very
little gpu activity (if any) that can stall.
v2:
- Fix the mock gtt selftest to bind with vma resources.
- Update a code comment.
- Account for rebinding the same vma with different I915_VMA_*_BIND flags
v3:
- Some style fixups.
- Move the sync fence wait to __i915_vma_evict instead of __i915_vma_unbind
to catch also the evict case on suspend.
v4:
- Remove a minor fix that incorrectly landed in this patch.
Signed-off-by: Thomas Hellström
---
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 2 +-
drivers/gpu/drm/i915/i915_vma.c | 206 +-
drivers/gpu/drm/i915/i915_vma.h | 20 +-
drivers/gpu/drm/i915/i915_vma_snapshot.c | 14 +-
drivers/gpu/drm/i915/i915_vma_snapshot.h | 2 +-
drivers/gpu/drm/i915/i915_vma_types.h | 5 +
drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 98 +
7 files changed, 288 insertions(+), 59 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 301eb58bebd1..69915c00ce18 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1376,7 +1376,7 @@ eb_relocate_entry(struct i915_execbuffer *eb,
GRAPHICS_VER(eb->i915) == 6) {
err = i915_vma_bind(target->vma,
target->vma->obj->cache_level,
- PIN_GLOBAL, NULL);
+ PIN_GLOBAL, NULL, NULL);
if (err)
return err;
}
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 90546fa58fc1..ff125cbea45c 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -38,7 +38,35 @@
#include "i915_trace.h"
#include "i915_vma.h"
+/**
+ * struct i915_vma_resource - Snapshotted unbind information.
+ * @unbind_fence: Fence to mark unbinding complete. Note that this fence
+ * is not considered published until unbind is scheduled, and as such it
+ * is illegal to access this fence before scheduled unbind other than
+ * for refcounting.
+ * @lock: The @unbind_fence lock. We're also using it to protect the weak
+ * pointer to the struct i915_vma, @vma during lookup and takedown.
+ * @vma: Weak back-pointer to the parent vma struct. This pointer is
+ * protected by @lock, and a reference on @vma needs to be taken
+ * using kref_get_unless_zero.
+ * @hold_count: Number of holders blocking the fence from finishing.
+ * The vma itself is keeping a hold, which is released when unbind
+ * is scheduled.
+ */
+struct i915_vma_resource {
+ struct dma_fence unbind_fence;
+ /* See above for description of the lock. */
+ spinlock_t lock;
+ struct i915_vma *vma;
+ refcount_t hold_count;
+};
+
static struct kmem_cache *slab_vmas;
+static struct dma_fence *i915_vma_resource_unbind(struct i915_vma_resource
*vma_res);
+#ifndef CONFIG_DRM_I915_SELFTEST
+static void i915_vma_resource_init(struct i915_vma_resource *vma_res,
+ struct i915_vma *vma);
+#endif
struct i915_vma *i915_vma_alloc(void)
{
@@ -363,6 +391,8 @@ int i915_vma_wait_for_bind(struct i915_vma *vma)
* @cache_level: mapping cache level
* @flags: flags like global or local mapping
* @work: preallocated worker for allocating and binding the PTE
+ * @vma_res: pointer to a preallocated vma resource. The resource is either
+ * consumed or freed.
*
* DMA addresses are taken from the scatter-gather table of this obj
Re: [PATCH v7 3/3] drm/bridge: anx7625: config hs packets end aligned to avoid screen shift
On Tue, 2 Nov 2021 at 02:56, Xin Ji wrote: > > On Mon, Nov 01, 2021 at 11:16:15AM +0800, Jitao Shi wrote: > > Hi Xin, > > > > Please help to review the changes in anx7625.c > > > > On Thu, 2021-09-16 at 06:31 +0800, Jitao Shi wrote: > > > This device requires the packets on lanes aligned at the end to fix > > > screen shift or scroll. > > > > > > Signed-off-by: Jitao Shi > > > --- > > > drivers/gpu/drm/bridge/analogix/anx7625.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c > > > b/drivers/gpu/drm/bridge/analogix/anx7625.c > > > index 14d73fb1dd15..d76fb63fa9f7 100644 > > > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c > > > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c > > > @@ -1327,6 +1327,7 @@ static int anx7625_attach_dsi(struct > > > anx7625_data *ctx) > > > MIPI_DSI_MODE_VIDEO_SYNC_PULSE | > > > MIPI_DSI_MODE_NO_EOT_PACKET | > > > MIPI_DSI_MODE_VIDEO_HSE; > > > + dsi->hs_packet_end_aligned = true; > > Looks good, it's OK for me. > Reviewed-by: Xin Ji Acked-by: Robert Foss
Re: [PATCH] backlight: led_bl: Add support for an "enable" GPIO
On Tue, Nov 02, 2021 at 10:04:55AM +, Corentin LABBE wrote:
> From: Jean-Jacques Hiblot
>
> This patch adds support for an "enable GPIO".
Before taking this kind of change is there a good reason why backlight
should manage the GPIO? I thought that the LED subsystem was a sub
system for LEDs (not LED controllers). In other words if you direct
that the LED be lit up then isn't it the LED driver's job to manage
the GPIO and ensure that it lights up?
Daniel.
> Signed-off-by: Jean-Jacques Hiblot
> Signed-off-by: Corentin LABBE
> ---
> drivers/video/backlight/led_bl.c | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/video/backlight/led_bl.c
> b/drivers/video/backlight/led_bl.c
> index f54d256e2d54..ebd7acc32980 100644
> --- a/drivers/video/backlight/led_bl.c
> +++ b/drivers/video/backlight/led_bl.c
> @@ -8,6 +8,7 @@
>
> #include
> #include
> +#include
> #include
> #include
>
> @@ -15,6 +16,7 @@ struct led_bl_data {
> struct device *dev;
> struct backlight_device *bl_dev;
> struct led_classdev **leds;
> + struct gpio_desc*enable_gpio;
> boolenabled;
> int nb_leds;
> unsigned int*levels;
> @@ -35,6 +37,9 @@ static void led_bl_set_brightness(struct led_bl_data *priv,
> int level)
> for (i = 0; i < priv->nb_leds; i++)
> led_set_brightness(priv->leds[i], bkl_brightness);
>
> + if (!priv->enabled)
> + gpiod_set_value_cansleep(priv->enable_gpio, 1);
> +
> priv->enabled = true;
> }
>
> @@ -48,6 +53,9 @@ static void led_bl_power_off(struct led_bl_data *priv)
> for (i = 0; i < priv->nb_leds; i++)
> led_set_brightness(priv->leds[i], LED_OFF);
>
> + if (priv->enabled)
> + gpiod_set_value_cansleep(priv->enable_gpio, 0);
> +
> priv->enabled = false;
> }
>
> @@ -209,6 +217,11 @@ static int led_bl_probe(struct platform_device *pdev)
> return PTR_ERR(priv->bl_dev);
> }
>
> + priv->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(priv->enable_gpio))
> + return PTR_ERR(priv->enable_gpio);
> +
> for (i = 0; i < priv->nb_leds; i++)
> led_sysfs_disable(priv->leds[i]);
>
> --
> 2.25.1
>
Re: [PATCH] backlight: led_bl: Add support for an "enable" GPIO
On Tue, Nov 02, 2021 at 11:19:42AM +, Daniel Thompson wrote:
> On Tue, Nov 02, 2021 at 10:04:55AM +, Corentin LABBE wrote:
> > From: Jean-Jacques Hiblot
> >
> > This patch adds support for an "enable GPIO".
>
> Before taking this kind of change is there a good reason why backlight
> should manage the GPIO? I thought that the LED subsystem was a sub
> system for LEDs (not LED controllers). In other words if you direct
> that the LED be lit up then isn't it the LED driver's job to manage
> the GPIO and ensure that it lights up?
Sorry... I should have paid more attention to my sense of déjà vu with
this patch.
This approach was discussed and rejected when we first introduced the
led_bl driver:
https://lore.kernel.org/linux-leds/20190705100851.zn2jkipj4fxq5we6@devuan/
Daniel.
> > Signed-off-by: Jean-Jacques Hiblot
> > Signed-off-by: Corentin LABBE
> > ---
> > drivers/video/backlight/led_bl.c | 13 +
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/drivers/video/backlight/led_bl.c
> > b/drivers/video/backlight/led_bl.c
> > index f54d256e2d54..ebd7acc32980 100644
> > --- a/drivers/video/backlight/led_bl.c
> > +++ b/drivers/video/backlight/led_bl.c
> > @@ -8,6 +8,7 @@
> >
> > #include
> > #include
> > +#include
> > #include
> > #include
> >
> > @@ -15,6 +16,7 @@ struct led_bl_data {
> > struct device *dev;
> > struct backlight_device *bl_dev;
> > struct led_classdev **leds;
> > + struct gpio_desc*enable_gpio;
> > boolenabled;
> > int nb_leds;
> > unsigned int*levels;
> > @@ -35,6 +37,9 @@ static void led_bl_set_brightness(struct led_bl_data
> > *priv, int level)
> > for (i = 0; i < priv->nb_leds; i++)
> > led_set_brightness(priv->leds[i], bkl_brightness);
> >
> > + if (!priv->enabled)
> > + gpiod_set_value_cansleep(priv->enable_gpio, 1);
> > +
> > priv->enabled = true;
> > }
> >
> > @@ -48,6 +53,9 @@ static void led_bl_power_off(struct led_bl_data *priv)
> > for (i = 0; i < priv->nb_leds; i++)
> > led_set_brightness(priv->leds[i], LED_OFF);
> >
> > + if (priv->enabled)
> > + gpiod_set_value_cansleep(priv->enable_gpio, 0);
> > +
> > priv->enabled = false;
> > }
> >
> > @@ -209,6 +217,11 @@ static int led_bl_probe(struct platform_device *pdev)
> > return PTR_ERR(priv->bl_dev);
> > }
> >
> > + priv->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
> > + GPIOD_OUT_LOW);
> > + if (IS_ERR(priv->enable_gpio))
> > + return PTR_ERR(priv->enable_gpio);
> > +
> > for (i = 0; i < priv->nb_leds; i++)
> > led_sysfs_disable(priv->leds[i]);
> >
> > --
> > 2.25.1
> >
Re: [Intel-gfx] [PATCH v3 05/10] drm/i915: Prepare for multiple gts
Hi Tvrtko, > > [...] > > > > > static int > > > intel_gt_tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t > > > phys_addr) > > > > we don't actually need 'id', it's gt->info.id. It's introduced in > > patch 3 with the value '0' but it's not needed. > > I have a suspicion code got munged up over endless rebases and refactors. > > This patch is the one which introduces the id member to gt->info. But it is > not setting it, even though I suspect the intent was for intel_gt_tile_setup > to do that. > > Instead gt->info.id is only set to a valid value in last patch of this series > inside intel_gt_probe_all: > > + gt->i915 = i915; > + gt->name = gtdef->name; > + gt->type = gtdef->type; > + gt->info.engine_mask = gtdef->engine_mask; > + gt->info.id = i; > + > + drm_dbg(&i915->drm, "Setting up %s %u\n", gt->name, > gt->info.id); > + ret = intel_gt_tile_setup(gt, i, phys_addr + > gtdef->mapping_base); > > And intel_gt_tile_setup then calls __intel_gt_init_early which assigns > gt->i915 yet again. > > So I'd say there is probably space to bring this all into a more streamlined > flow, even more than what you suggest below. yes, I noticed them! Patch 3, 5 and 10 are very much connected with each other: 3 prepares for one tile, 5 prepares for multitile and 10 does the multitile. While in between other patches are doing other things. On top of some cleanups we could also rearrange the patches with some squashing and reordering to have them a bit more linear and also easier to review. Andi
[PATCH] drm/virtio: delay pinning the pages till first use
From: mwezdeck
The idea behind the commit:
1. not pin the pages during resource_create ioctl
2. pin the pages on the first use during:
- transfer_*_host ioctl
- map ioctl
3. introduce new ioctl for pinning pages on demand
This change has no impact on user space.
Signed-off-by: mwezdeck
---
drivers/gpu/drm/virtio/virtgpu_drv.h| 6 ++-
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 65 +
drivers/gpu/drm/virtio/virtgpu_object.c | 42
include/uapi/drm/virtgpu_drm.h | 10
4 files changed, 113 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index e0265fe74aa5..cf2cad663575 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -278,7 +278,7 @@ struct virtio_gpu_fpriv {
};
/* virtgpu_ioctl.c */
-#define DRM_VIRTIO_NUM_IOCTLS 12
+#define DRM_VIRTIO_NUM_IOCTLS 13
extern struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS];
void virtio_gpu_create_context(struct drm_device *dev, struct drm_file *file);
@@ -455,6 +455,10 @@ int virtio_gpu_object_create(struct virtio_gpu_device
*vgdev,
struct virtio_gpu_object **bo_ptr,
struct virtio_gpu_fence *fence);
+int virtio_gpu_object_pin(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_object_array *objs,
+ int num_gem_objects);
+
bool virtio_gpu_is_shmem(struct virtio_gpu_object *bo);
int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev,
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index 5618a1d5879c..49bf53f358b5 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -102,6 +102,25 @@ static int virtio_gpu_map_ioctl(struct drm_device *dev,
void *data,
{
struct virtio_gpu_device *vgdev = dev->dev_private;
struct drm_virtgpu_map *virtio_gpu_map = data;
+ struct virtio_gpu_object_array *objs;
+ struct virtio_gpu_object *bo;
+ struct virtio_gpu_object_shmem *shmem;
+
+ objs = virtio_gpu_array_from_handles(file, &virtio_gpu_map->handle, 1);
+ if (objs == NULL)
+ return -ENOENT;
+
+ bo = gem_to_virtio_gpu_obj(objs->objs[0]);
+ if (bo == NULL)
+ return -ENOENT;
+
+ shmem = to_virtio_gpu_shmem(bo);
+ if (shmem == NULL)
+ return -ENOENT;
+
+ if (!shmem->pages) {
+ virtio_gpu_object_pin(vgdev, objs, 1);
+ }
return virtio_gpu_mode_dumb_mmap(file, vgdev->ddev,
virtio_gpu_map->handle,
@@ -292,6 +311,9 @@ static int virtio_gpu_getparam_ioctl(struct drm_device
*dev, void *data,
case VIRTGPU_PARAM_SUPPORTED_CAPSET_IDs:
value = vgdev->capset_id_mask;
break;
+ case VIRTGPU_PARAM_PIN_ON_DEMAND:
+ value = 1;
+ break;
default:
return -EINVAL;
}
@@ -397,6 +419,7 @@ static int virtio_gpu_transfer_from_host_ioctl(struct
drm_device *dev,
struct virtio_gpu_object *bo;
struct virtio_gpu_object_array *objs;
struct virtio_gpu_fence *fence;
+ struct virtio_gpu_object_shmem *shmem;
int ret;
u32 offset = args->offset;
@@ -414,6 +437,11 @@ static int virtio_gpu_transfer_from_host_ioctl(struct
drm_device *dev,
goto err_put_free;
}
+ shmem = to_virtio_gpu_shmem(bo);
+ if (!shmem->pages) {
+ virtio_gpu_object_pin(vgdev, objs, 1);
+ }
+
if (!bo->host3d_blob && (args->stride || args->layer_stride)) {
ret = -EINVAL;
goto err_put_free;
@@ -451,6 +479,7 @@ static int virtio_gpu_transfer_to_host_ioctl(struct
drm_device *dev, void *data,
struct drm_virtgpu_3d_transfer_to_host *args = data;
struct virtio_gpu_object *bo;
struct virtio_gpu_object_array *objs;
+ struct virtio_gpu_object_shmem *shmem;
struct virtio_gpu_fence *fence;
int ret;
u32 offset = args->offset;
@@ -465,6 +494,11 @@ static int virtio_gpu_transfer_to_host_ioctl(struct
drm_device *dev, void *data,
goto err_put_free;
}
+ shmem = to_virtio_gpu_shmem(bo);
+ if (!shmem->pages) {
+ virtio_gpu_object_pin(vgdev, objs, 1);
+ }
+
if (!vgdev->has_virgl_3d) {
virtio_gpu_cmd_transfer_to_host_2d
(vgdev, offset,
@@ -836,6 +870,34 @@ static int virtio_gpu_context_init_ioctl(struct drm_device
*dev,
return ret;
}
+static int virtio_gpu_pin_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file)
+{
+ struct virtio_gpu_device *vgdev = dev->dev_private;
+ struct drm_virtgpu_pi
[Bug 214901] amdgpu freezes HP laptop at start up
https://bugzilla.kernel.org/show_bug.cgi?id=214901 --- Comment #6 from t...@siduction.org --- Looks like the same problem as i stated here: https://bugzilla.kernel.org/show_bug.cgi?id=214859 -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH 3/3] backlight: lp855x: Add support ACPI enumeration
On Mon, Nov 01, 2021 at 07:55:17PM +0100, Hans de Goede wrote:
> The Xiaomi Mi Pad 2 tablet uses an ACPI enumerated LP8556 backlight
> controller for its LCD-panel, with a Xiaomi specific ACPI HID of
> "XMCC0001", add support for this.
>
> Note the new "if (id)" check also fixes a NULL pointer deref when a user
> tries to manually bind the driver from sysfs.
>
> When CONFIG_ACPI is disabled acpi_match_device() will always return NULL,
> so the lp855x_parse_acpi() call will get optimized away.
>
> Signed-off-by: Hans de Goede
> ---
> drivers/video/backlight/lp855x_bl.c | 70 -
> 1 file changed, 60 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/video/backlight/lp855x_bl.c
> b/drivers/video/backlight/lp855x_bl.c
> index d1d27d5eb0f2..f075ec84acfb 100644
> --- a/drivers/video/backlight/lp855x_bl.c
> +++ b/drivers/video/backlight/lp855x_bl.c
> @@ -338,10 +339,6 @@ static int lp855x_parse_dt(struct lp855x *lp)
> return -EINVAL;
> }
>
> - pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> - if (!pdata)
> - return -ENOMEM;
> -
> of_property_read_string(node, "bl-name", &pdata->name);
> of_property_read_u8(node, "dev-ctrl", &pdata->device_control);
> of_property_read_u8(node, "init-brt", &pdata->initial_brightness);
Shouldn't there be a removal of an `lp->pdata = pdata` from somewhere in
this function?
> @@ -379,8 +376,31 @@ static int lp855x_parse_dt(struct lp855x *lp)
> }
> #endif
>
> +static int lp855x_parse_acpi(struct lp855x *lp)
> +{
> + int ret;
> +
> + /*
> + * On ACPI the device has already been initialized by the firmware
Perhaps nitpicking but ideally I'd like "and is in register mode" here
since I presume it can also be assumed that everything with this HID
has adopted that).
Other than these, LGTM.
Daniel.
Re: [PATCH 2/3] backlight: lp855x: Add dev helper variable to lp855x_probe()
On Mon, Nov 01, 2021 at 07:55:16PM +0100, Hans de Goede wrote: > Add a dev local variable to the lp855x_probe(), to replace "&cl->dev" > and "lp->dev" in various places. > > Also switch to dev_err_probe() in one case which takes care of not > printing -EPROBE_DEFER errors for us. > > This is mostly a preparation for adding ACPI enumeration support which > will use the new "dev" variable more. > > Signed-off-by: Hans de Goede Reviewed-by: Daniel Thompson Daniel.
Re: [PATCH 1/3] backlight: lp855x: Move device_config setting out of lp855x_configure()
On Mon, Nov 01, 2021 at 07:55:15PM +0100, Hans de Goede wrote: > Move the setting of the lp->cfg pointer to the chip specific > lp855x_device_config struct from lp855x_configure() to > lp855x_probe(), before calling lp855x_parse_dt(). > > This is a preperation patch for adding ACPI enumeration support. > > Signed-off-by: Hans de Goede Reviewed-by: Daniel Thompson Daniel.
Re: [PATCH v3 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
On Mon, Oct 25, 2021 at 12:17:37AM +0200, Claudio Suarez wrote: [...] No new comments about this, I suppose everything is fine. I'm going to send the patch with this changes. Thanks to all and special thanks to you, Ville. Hope this helps the kernel. Don't hesitate to ask new changes if necessary. Best regards Claudio Suarez
Re: [PATCH v4 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Commit a92d083d08b0 created the new flag is_hdmi in drm_display_info
which is set when sink compliant with CEA-861 (EDID) will be treated
as an HDMI sink.
>From that day, this value can be used in some cases instead of
calling drm_detect_hdmi_monitor() and a second parse is avoided
because drm_detect_hdmi_monitor() parses. A TODO task was
registered in Documentation/gpu/todo.rst to perform that task in
the future.
The flag drm_display_info.is_hdmi is set in the function
drm_add_display_info(), which is called from
drm_connector_update_edid_property(). Since commit 5186421cbfe2,
drm_get_edid() calls drm_connector_update_edid_property() when
reading the edid data from an i2c adapter. Therefore, in these
cases drm_display_info.is_hdmi is updated to its correct
value when returning from drm_get_edid().
Replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
in the cases when drm_detect_hdmi_monitor() is called after a
read from an i2c adapter using drm_get_edid() in the i915 driver.
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index b04685bb6439..008e5b0ba408 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2355,7 +2355,7 @@ intel_hdmi_set_edid(struct drm_connector *connector)
to_intel_connector(connector)->detect_edid = edid;
if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
- intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
+ intel_hdmi->has_hdmi_sink = connector->display_info.is_hdmi;
connected = true;
}
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
b/drivers/gpu/drm/i915/display/intel_sdvo.c
index 6cb27599ea03..b4065e4df644 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -2060,8 +2060,9 @@ intel_sdvo_tmds_sink_detect(struct drm_connector
*connector)
if (edid->input & DRM_EDID_INPUT_DIGITAL) {
status = connector_status_connected;
if (intel_sdvo_connector->is_hdmi) {
- intel_sdvo->has_hdmi_monitor =
drm_detect_hdmi_monitor(edid);
intel_sdvo->has_hdmi_audio =
drm_detect_monitor_audio(edid);
+ intel_sdvo->has_hdmi_monitor =
+
connector->display_info.is_hdmi;
}
} else
status = connector_status_disconnected;
--
2.33.0
Re: [PATCH] drm/virtio: delay pinning the pages till first use
On Tue, Nov 02, 2021 at 12:31:39PM +0100, Maksym Wezdecki wrote:
> From: mwezdeck
>
> The idea behind the commit:
> 1. not pin the pages during resource_create ioctl
> 2. pin the pages on the first use during:
> - transfer_*_host ioctl
> - map ioctl
i.e. basically lazy pinning. Approach looks sane to me.
> 3. introduce new ioctl for pinning pages on demand
What is the use case for this ioctl?
In any case this should be a separate patch.
> + struct virtio_gpu_object_array *objs;
> + struct virtio_gpu_object *bo;
> + struct virtio_gpu_object_shmem *shmem;
> +
> + objs = virtio_gpu_array_from_handles(file, &virtio_gpu_map->handle, 1);
> + if (objs == NULL)
> + return -ENOENT;
> +
> + bo = gem_to_virtio_gpu_obj(objs->objs[0]);
> + if (bo == NULL)
> + return -ENOENT;
> +
> + shmem = to_virtio_gpu_shmem(bo);
> + if (shmem == NULL)
> + return -ENOENT;
> +
> + if (!shmem->pages) {
> + virtio_gpu_object_pin(vgdev, objs, 1);
> + }
Move this into virtio_gpu_object_pin(),
or create a helper function for it ...
> + objs = virtio_gpu_array_from_handles(file, &virtio_gpu_pin->handle, 1);
> + if (objs == NULL)
> + return -ENOENT;
> +
> + bo = gem_to_virtio_gpu_obj(objs->objs[0]);
> + if (bo == NULL)
> + return -ENOENT;
> +
> + shmem = to_virtio_gpu_shmem(bo);
> + if (shmem == NULL)
> + return -ENOENT;
> +
> + if (!shmem->pages) {
> + return virtio_gpu_object_pin(vgdev, objs, 1);
> + }
... to avoid this code duplication?
> +int virtio_gpu_object_pin(struct virtio_gpu_device *vgdev,
> + struct virtio_gpu_object_array *objs,
> + int num_gem_objects)
> +{
> + int i, ret;
> +
> + for (i = 0; i < num_gem_objects; i++) {
> + ret = virtio_gpu_object_shmem_init(vgdev, bo, &ents, &nents);
> + if (ret != 0) {
> + return -EFAULT;
> + }
> +
> + virtio_gpu_object_attach(vgdev, bo, ents, nents);
I think it is enough to do the virtio_gpu_object_attach() call lazily.
virtio_gpu_object_shmem_init() should not actually allocate pages, that
only happens when virtio_gpu_object_attach() goes ask for a scatter
list.
take care,
Gerd
[PATCH] drm/bridge: parade-ps8640: Link device to ensure suspend/resume order
Entering suspend while the display attached to this bridge is still on
makes the resume sequence to resume the bridge first, display last:
when this happens, we get a timeout while resuming the bridge, as its
MCU will get stuck due to the display being unpowered.
On the other hand, on mt8173-elm, closing the lid makes the display to
get powered off first, bridge last, so at resume time the sequence is
swapped (compared to the first example) and everything just works
as expected.
Add a stateless device link to the DRM device that this bridge belongs
to, ensuring a correct resume sequence and solving the unability to
correctly resume bridge operation in the first mentioned example.
Signed-off-by: AngeloGioacchino Del Regno
---
drivers/gpu/drm/bridge/parade-ps8640.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c
b/drivers/gpu/drm/bridge/parade-ps8640.c
index 45100edd745b..191cc196c9d1 100644
--- a/drivers/gpu/drm/bridge/parade-ps8640.c
+++ b/drivers/gpu/drm/bridge/parade-ps8640.c
@@ -100,6 +100,7 @@ struct ps8640 {
struct regulator_bulk_data supplies[2];
struct gpio_desc *gpio_reset;
struct gpio_desc *gpio_powerdown;
+ struct device_link *link;
bool powered;
};
@@ -460,10 +461,23 @@ static int ps8640_bridge_attach(struct drm_bridge *bridge,
goto err_aux_register;
}
+ ps_bridge->link = device_link_add(bridge->dev->dev, dev,
DL_FLAG_STATELESS);
+ if (!ps_bridge->link) {
+ dev_err(dev, "failed to create device link");
+ ret = -EINVAL;
+ goto err_devlink;
+ }
+
/* Attach the panel-bridge to the dsi bridge */
- return drm_bridge_attach(bridge->encoder, ps_bridge->panel_bridge,
+ ret = drm_bridge_attach(bridge->encoder, ps_bridge->panel_bridge,
&ps_bridge->bridge, flags);
+ if (ret)
+ goto err_bridge_attach;
+err_bridge_attach:
+ device_link_del(ps_bridge->link);
+err_devlink:
+ drm_dp_aux_unregister(&ps_bridge->aux);
err_aux_register:
mipi_dsi_detach(dsi);
err_dsi_attach:
@@ -473,7 +487,11 @@ static int ps8640_bridge_attach(struct drm_bridge *bridge,
static void ps8640_bridge_detach(struct drm_bridge *bridge)
{
- drm_dp_aux_unregister(&bridge_to_ps8640(bridge)->aux);
+ struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
+
+ drm_dp_aux_unregister(&ps_bridge->aux);
+ if (ps_bridge->link)
+ device_link_del(ps_bridge->link);
}
static struct edid *ps8640_bridge_get_edid(struct drm_bridge *bridge,
--
2.33.1
Re: [PATCH v8, 15/17] dt-bindings: media: mtk-vcodec: Adds decoder dt-bindings for mt8192
On Fri, Oct 29, 2021 at 11:55:25AM +0800, Yunfei Dong wrote: > Adds decoder dt-bindings for mt8192. > > Signed-off-by: Yunfei Dong > --- > v8: fix yaml file check fail > --- > .../media/mediatek,vcodec-comp-decoder.yaml | 273 ++ > 1 file changed, 273 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/media/mediatek,vcodec-comp-decoder.yaml > > diff --git > a/Documentation/devicetree/bindings/media/mediatek,vcodec-comp-decoder.yaml > b/Documentation/devicetree/bindings/media/mediatek,vcodec-comp-decoder.yaml > new file mode 100644 > index ..40a076756439 > --- /dev/null > +++ > b/Documentation/devicetree/bindings/media/mediatek,vcodec-comp-decoder.yaml > @@ -0,0 +1,273 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > + > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/media/mediatek,vcodec-comp-decoder.yaml#"; > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"; > + > +title: Mediatek Video Decode Accelerator With Multi Hardware > + > +maintainers: > + - Yunfei Dong > + > +description: | > + Mediatek Video Decode is the video decode hardware present in Mediatek > + SoCs which supports high resolution decoding functionalities. Required > + master and component node. > + > + About the Decoder Hardware Block Diagram, please check below: > + > ++-++ > +| || > +| input -> lat HW -> lat buffer --|--> lat buffer -> core HW -> output | > +||| | || | > ++||---+-||-+ > + || lat thread | core thread|| > + > -||-|| > + || || > > + \/\/ > + +--+ > + |enable/disable| > + | clk powerirqiommu port | > + | (lat/lat soc/core0/core1)| > + +--+ > + > + As above, mean in master device, mean in component > device. > + The information of each hardware will be stored in each component device. > There > + are two workqueue in master device: lat and core. Enable/disable the lat > clk/power/irq > + when lat hardware need to work through hardware index, core is the same. > + > + Normally the smi common may not the same for each hardware, can't combine > all > + hardware in one node, or leading to iommu fault when access dram data. > + > +properties: > + compatible: > +const: mediatek,mt8192-vcodec-dec > + > + reg: > +maxItems: 1 > + > + iommus: > +minItems: 1 > +maxItems: 32 > +description: | > + List of the hardware port in respective IOMMU block for current Socs. > + Refer to bindings/iommu/mediatek,iommu.yaml. > + > + mediatek,scp: > +$ref: /schemas/types.yaml#/definitions/phandle > +maxItems: 1 > +description: | > + The node of system control processor (SCP), using > + the remoteproc & rpmsg framework. > + $ref: /schemas/remoteproc/mtk,scp.yaml > + > + dma-ranges: > +maxItems: 1 > +description: | > + Describes the physical address space of IOMMU maps to memory. > + > + "#address-cells": > +const: 1 > + > + "#size-cells": > +const: 1 > + > + ranges: true > + > +# Required child node: > +patternProperties: > + vcodec-lat: > +type: object > + > +properties: > + compatible: > +const: mediatek,mtk-vcodec-lat > + > + reg: > +maxItems: 1 > + > + reg-names: > +maxItems: 1 You have to document what the names are. But 'misc' is isn't really specific and you don't need -names when there is only 1, so I'd just drop it. > + > + interrupts: > +maxItems: 1 > + > + iommus: > +minItems: 1 > +maxItems: 32 > +description: | > + List of the hardware port in respective IOMMU block for current > Socs. > + Refer to bindings/iommu/mediatek,iommu.yaml. > + > + clocks: > +maxItems: 5 > + > + clock-names: > +items: > + - const: vdec-sel > + - const: vdec-soc-vdec > + - const: vdec-soc-lat > + - const: vdec-vdec > + - const: vdec-top 'vdec-' is redundant. Names are local to the node. > + > + assigned-clocks: > +maxItems: 1 > + > + assigned-clock-parents: > +maxItems: 1 > + > + power-domains: > +maxItems: 1 > + > +required: > + - compatible > + - reg > + - reg-names > + - interr
Re: [PATCH v2 2/2] drm/i915/ttm: Failsafe migration blits
On 01/11/2021 18:38, Thomas Hellström wrote:
If the initial fill blit or copy blit of an object fails, the old
content of the data might be exposed and read as soon as either CPU- or
GPU PTEs are set up to point at the pages.
Intercept the blit fence with an async callback that checks the
blit fence for errors and if there are errors performs an async cpu blit
instead. If there is a failure to allocate the async dma_fence_work,
allocate it on the stack and sync wait for the blit to complete.
Add selftests that simulate gpu blit failures and failure to allocate
the async dma_fence_work.
A previous version of this pach used dma_fence_work, now that's
opencoded which adds more code but might lower the latency
somewhat in the common non-error case.
Signed-off-by: Thomas Hellström
---
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 322 +++---
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h | 5 +
.../drm/i915/gem/selftests/i915_gem_migrate.c | 24 +-
3 files changed, 295 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
index 0ed6b7f2b95f..2e3328e2b48c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -18,6 +18,29 @@
#include "gt/intel_gt.h"
#include "gt/intel_migrate.h"
+/**
+ * DOC: Selftest failure modes for failsafe migration:
+ *
+ * For fail_gpu_migration, the gpu blit scheduled is always a clear blit
+ * rather than a copy blit, and then we force the failure paths as if
+ * the blit fence returned an error.
+ *
+ * For fail_work_allocation we fail the kmalloc of the async worker, we
+ * sync the gpu blit. If it then fails, or fail_gpu_migration is set to
+ * true, then a memcpy operation is performed sync.
+ */
+I915_SELFTEST_DECLARE(static bool fail_gpu_migration;)
+I915_SELFTEST_DECLARE(static bool fail_work_allocation;)
Might as well move these under the CONFIG_SELFTEST below, and then drop
the DECLARE stuff?
+
+#ifdef CONFIG_DRM_I915_SELFTEST
+void i915_ttm_migrate_set_failure_modes(bool gpu_migration,
+ bool work_allocation)
+{
+ fail_gpu_migration = gpu_migration;
+ fail_work_allocation = work_allocation;
+}
+#endif
+
static enum i915_cache_level
i915_ttm_cache_level(struct drm_i915_private *i915, struct ttm_resource *res,
struct ttm_tt *ttm)
@@ -129,11 +152,11 @@ int i915_ttm_move_notify(struct ttm_buffer_object *bo)
return 0;
}
-static int i915_ttm_accel_move(struct ttm_buffer_object *bo,
- bool clear,
- struct ttm_resource *dst_mem,
- struct ttm_tt *dst_ttm,
- struct sg_table *dst_st)
+static struct dma_fence *i915_ttm_accel_move(struct ttm_buffer_object *bo,
+bool clear,
+struct ttm_resource *dst_mem,
+struct ttm_tt *dst_ttm,
+struct sg_table *dst_st)
{
struct drm_i915_private *i915 = container_of(bo->bdev, typeof(*i915),
bdev);
@@ -144,30 +167,29 @@ static int i915_ttm_accel_move(struct ttm_buffer_object
*bo,
int ret;
if (!i915->gt.migrate.context || intel_gt_is_wedged(&i915->gt))
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
+
+ /* With fail_gpu_migration, we always perform a GPU clear. */
+ if (I915_SELFTEST_ONLY(fail_gpu_migration))
+ clear = true;
dst_level = i915_ttm_cache_level(i915, dst_mem, dst_ttm);
if (clear) {
- if (bo->type == ttm_bo_type_kernel)
- return -EINVAL;
+ if (bo->type == ttm_bo_type_kernel &&
+ !I915_SELFTEST_ONLY(fail_gpu_migration))
+ return ERR_PTR(-EINVAL);
intel_engine_pm_get(i915->gt.migrate.context->engine);
ret = intel_context_migrate_clear(i915->gt.migrate.context,
NULL,
dst_st->sgl, dst_level,
i915_ttm_gtt_binds_lmem(dst_mem),
0, &rq);
-
- if (!ret && rq) {
- i915_request_wait(rq, 0, MAX_SCHEDULE_TIMEOUT);
- i915_request_put(rq);
- }
- intel_engine_pm_put(i915->gt.migrate.context->engine);
} else {
struct i915_refct_sgt *src_rsgt =
i915_ttm_resource_get_st(obj, bo->resource);
if (IS_ERR(src_rsgt))
- return PTR_ERR(src_rsgt);
+ return ERR_CAST(src_rsgt);
src_level = i915_ttm_cache_level(i915, bo->resource, s
Re: [PATCH 02/29] drm/i915/gvt: integrate into the main Makefile
On Tue, Nov 02, 2021 at 08:05:34AM +0100, Christoph Hellwig wrote: > Remove the separately included Makefile and just use the relative > reference from the main i915 Makefile as for source files in other > subdirectories. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/Makefile | 29 - > drivers/gpu/drm/i915/gvt/Makefile | 9 - > drivers/gpu/drm/i915/gvt/trace.h | 2 +- > 3 files changed, 25 insertions(+), 15 deletions(-) > delete mode 100644 drivers/gpu/drm/i915/gvt/Makefile Reviewed-by: Jason Gunthorpe > - > -ifeq ($(CONFIG_DRM_I915_GVT),y) > -i915-y += intel_gvt.o > -include $(src)/gvt/Makefile > -endif > +i915-$(CONFIG_DRM_I915_GVT) += \ > + intel_gvt.o \ > + gvt/gvt.o \ > + gvt/aperture_gm.o \ > + gvt/handlers.o \ > + gvt/vgpu.o \ > + gvt/trace_points.o \ > + gvt/firmware.o \ > + gvt/interrupt.o \ > + gvt/gtt.o \ > + gvt/cfg_space.o \ > + gvt/opregion.o \ > + gvt/mmio.o \ > + gvt/display.o \ > + gvt/edid.o \ > + gvt/execlist.o \ > + gvt/scheduler.o \ > + gvt/sched_policy.o \ > + gvt/mmio_context.o \ > + gvt/cmd_parser.o \ > + gvt/debugfs.o \ > + gvt/fb_decoder.o \ > + gvt/dmabuf.o \ > + gvt/page_track.o nit, nicer to sort makefile lists Jason
Re: [Intel-gfx] [PATCH v3 05/10] drm/i915: Prepare for multiple gts
On 02/11/2021 11:26, Andi Shyti wrote: Hi Tvrtko, [...] static int intel_gt_tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t phys_addr) we don't actually need 'id', it's gt->info.id. It's introduced in patch 3 with the value '0' but it's not needed. I have a suspicion code got munged up over endless rebases and refactors. This patch is the one which introduces the id member to gt->info. But it is not setting it, even though I suspect the intent was for intel_gt_tile_setup to do that. Instead gt->info.id is only set to a valid value in last patch of this series inside intel_gt_probe_all: + gt->i915 = i915; + gt->name = gtdef->name; + gt->type = gtdef->type; + gt->info.engine_mask = gtdef->engine_mask; + gt->info.id = i; + + drm_dbg(&i915->drm, "Setting up %s %u\n", gt->name, gt->info.id); + ret = intel_gt_tile_setup(gt, i, phys_addr + gtdef->mapping_base); And intel_gt_tile_setup then calls __intel_gt_init_early which assigns gt->i915 yet again. So I'd say there is probably space to bring this all into a more streamlined flow, even more than what you suggest below. yes, I noticed them! Patch 3, 5 and 10 are very much connected with each other: 3 prepares for one tile, 5 prepares for multitile and 10 does the multitile. While in between other patches are doing other things. On top of some cleanups we could also rearrange the patches with some squashing and reordering to have them a bit more linear and also easier to review. Yes. Maybe make intel_gt_tile_setup accept more arguments so it can be truly used to setup a gt? intel_gt_tile_setup(gt, id, name, type, engine_mask) The usual thing where patch which adds something extends the prototype to include more stuff. If that applies here. I know it is originally my patch but I don't have the time to rework it, much less the whole series, so usual dispensation to take over authorship if changes are large applies. Regards, Tvrtko
Re: [PATCH v2 06/13] drm/exynos: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
On Wed, Oct 27, 2021 at 07:28:45AM +0900, Inki Dae wrote: > Hi, > > 21. 10. 17. 오전 3:42에 Claudio Suarez 이(가) 쓴 글: > > Once EDID is parsed, the monitor HDMI support information is available > > through drm_display_info.is_hdmi. Retriving the same information with > > drm_detect_hdmi_monitor() is less efficient. Change to > > drm_display_info.is_hdmi > > > > Signed-off-by: Claudio Suarez > > --- > > drivers/gpu/drm/exynos/exynos_hdmi.c | 6 -- > > 1 file changed, 4 insertions(+), 2 deletions( > > > > diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c > > b/drivers/gpu/drm/exynos/exynos_hdmi.c > > index 7655142a4651..a563d6386abe 100644 > > --- a/drivers/gpu/drm/exynos/exynos_hdmi.c > > +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c > > @@ -893,12 +893,14 @@ static int hdmi_get_modes(struct drm_connector > > *connector) > > if (!edid) > > return -ENODEV; > > > > - hdata->dvi_mode = !drm_detect_hdmi_monitor(edid); > > + /* This updates connector->display_info */ > > + drm_connector_update_edid_property(connector, edid); > > + > > + hdata->dvi_mode = !connector->display_info.is_hdmi; > > Thanks for correcting this. Yeah, we should use drm_display_info.is_hdmi > parsed from EDID. > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpu/drm/drm_edid.c?h=v5.14.14#n4725 > > Signed-off-by: Inki Dae Thank you, Inki. Best regards Claudio Suarez
Re: [PATCH v2 2/2] drm/i915/ttm: Failsafe migration blits
Thanks for reviewing Matt,
On 11/2/21 14:55, Matthew Auld wrote:
On 01/11/2021 18:38, Thomas Hellström wrote:
If the initial fill blit or copy blit of an object fails, the old
content of the data might be exposed and read as soon as either CPU- or
GPU PTEs are set up to point at the pages.
Intercept the blit fence with an async callback that checks the
blit fence for errors and if there are errors performs an async cpu blit
instead. If there is a failure to allocate the async dma_fence_work,
allocate it on the stack and sync wait for the blit to complete.
Add selftests that simulate gpu blit failures and failure to allocate
the async dma_fence_work.
A previous version of this pach used dma_fence_work, now that's
opencoded which adds more code but might lower the latency
somewhat in the common non-error case.
Signed-off-by: Thomas Hellström
---
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 322 +++---
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h | 5 +
.../drm/i915/gem/selftests/i915_gem_migrate.c | 24 +-
3 files changed, 295 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
index 0ed6b7f2b95f..2e3328e2b48c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -18,6 +18,29 @@
#include "gt/intel_gt.h"
#include "gt/intel_migrate.h"
+/**
+ * DOC: Selftest failure modes for failsafe migration:
+ *
+ * For fail_gpu_migration, the gpu blit scheduled is always a clear
blit
+ * rather than a copy blit, and then we force the failure paths as if
+ * the blit fence returned an error.
+ *
+ * For fail_work_allocation we fail the kmalloc of the async worker, we
+ * sync the gpu blit. If it then fails, or fail_gpu_migration is set to
+ * true, then a memcpy operation is performed sync.
+ */
+I915_SELFTEST_DECLARE(static bool fail_gpu_migration;)
+I915_SELFTEST_DECLARE(static bool fail_work_allocation;)
Might as well move these under the CONFIG_SELFTEST below, and then
drop the DECLARE stuff?
+
+#ifdef CONFIG_DRM_I915_SELFTEST
+void i915_ttm_migrate_set_failure_modes(bool gpu_migration,
+ bool work_allocation)
+{
+ fail_gpu_migration = gpu_migration;
+ fail_work_allocation = work_allocation;
+}
+#endif
+
static enum i915_cache_level
i915_ttm_cache_level(struct drm_i915_private *i915, struct
ttm_resource *res,
struct ttm_tt *ttm)
@@ -129,11 +152,11 @@ int i915_ttm_move_notify(struct
ttm_buffer_object *bo)
return 0;
}
-static int i915_ttm_accel_move(struct ttm_buffer_object *bo,
- bool clear,
- struct ttm_resource *dst_mem,
- struct ttm_tt *dst_ttm,
- struct sg_table *dst_st)
+static struct dma_fence *i915_ttm_accel_move(struct
ttm_buffer_object *bo,
+ bool clear,
+ struct ttm_resource *dst_mem,
+ struct ttm_tt *dst_ttm,
+ struct sg_table *dst_st)
{
struct drm_i915_private *i915 = container_of(bo->bdev,
typeof(*i915),
bdev);
@@ -144,30 +167,29 @@ static int i915_ttm_accel_move(struct
ttm_buffer_object *bo,
int ret;
if (!i915->gt.migrate.context || intel_gt_is_wedged(&i915->gt))
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
+
+ /* With fail_gpu_migration, we always perform a GPU clear. */
+ if (I915_SELFTEST_ONLY(fail_gpu_migration))
+ clear = true;
dst_level = i915_ttm_cache_level(i915, dst_mem, dst_ttm);
if (clear) {
- if (bo->type == ttm_bo_type_kernel)
- return -EINVAL;
+ if (bo->type == ttm_bo_type_kernel &&
+ !I915_SELFTEST_ONLY(fail_gpu_migration))
+ return ERR_PTR(-EINVAL);
intel_engine_pm_get(i915->gt.migrate.context->engine);
ret = intel_context_migrate_clear(i915->gt.migrate.context,
NULL,
dst_st->sgl, dst_level,
i915_ttm_gtt_binds_lmem(dst_mem),
0, &rq);
-
- if (!ret && rq) {
- i915_request_wait(rq, 0, MAX_SCHEDULE_TIMEOUT);
- i915_request_put(rq);
- }
- intel_engine_pm_put(i915->gt.migrate.context->engine);
} else {
struct i915_refct_sgt *src_rsgt =
i915_ttm_resource_get_st(obj, bo->resource);
if (IS_ERR(src_rsgt))
- return PTR_ERR(src_rsgt);
+ return ERR_CAST(src_rsgt);
src_level = i915_ttm_cache_level(i915, bo->resource,
src_ttm);
intel_engine_pm_get(i915->gt.migrate.context->engine);
@@ -178,15 +200,183 @@ static int i915_ttm_accel_move(struct
ttm_buffer_object *bo,
dst_st->sgl, dst_level,
i915_ttm_gtt_binds_lmem(dst_mem),
&rq);
+
i915_refct_sgt_put(src_rsg
[PATCH v3 1/3] arm64: dts: qcom: sc7280: add display dt nodes
From: Krishna Manikandan
Add mdss and mdp DT nodes for sc7280.
Signed-off-by: Krishna Manikandan
Reported-by: kernel test robot
Reviewed-by: Stephen Boyd
Reported-by: kernel test robot
Signed-off-by: Sankeerth Billakanti
---
Changes in v3:
None
Changes in v2:
- Rename display dt nodes (Stephen Boyd)
- Add clock names one per line for readability (Stephen Boyd)
arch/arm64/boot/dts/qcom/sc7280.dtsi | 90
1 file changed, 90 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 365a2e0..a4536b6 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2704,6 +2704,96 @@
#power-domain-cells = <1>;
};
+ mdss: display-subsystem@ae0 {
+ compatible = "qcom,sc7280-mdss";
+ reg = <0 0x0ae0 0 0x1000>;
+ reg-names = "mdss";
+
+ power-domains = <&dispcc DISP_CC_MDSS_CORE_GDSC>;
+
+ clocks = <&gcc GCC_DISP_AHB_CLK>,
+<&dispcc DISP_CC_MDSS_AHB_CLK>,
+ <&dispcc DISP_CC_MDSS_MDP_CLK>;
+ clock-names = "iface",
+ "ahb",
+ "core";
+
+ assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>;
+ assigned-clock-rates = <3>;
+
+ interrupts = ;
+ interrupt-controller;
+ #interrupt-cells = <1>;
+
+ interconnects = <&mmss_noc MASTER_MDP0 0 &mc_virt
SLAVE_EBI1 0>;
+ interconnect-names = "mdp0-mem";
+
+ iommus = <&apps_smmu 0x900 0x402>;
+
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ status = "disabled";
+
+ mdp: display-controller@ae01000 {
+ compatible = "qcom,sc7280-dpu";
+ reg = <0 0x0ae01000 0 0x8f030>,
+ <0 0x0aeb 0 0x2008>;
+ reg-names = "mdp", "vbif";
+
+ clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
+ <&gcc GCC_DISP_SF_AXI_CLK>,
+ <&dispcc DISP_CC_MDSS_AHB_CLK>,
+ <&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
+ <&dispcc DISP_CC_MDSS_MDP_CLK>,
+ <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
+ clock-names = "bus",
+ "nrt_bus",
+ "iface",
+ "lut",
+ "core",
+ "vsync";
+ assigned-clocks = <&dispcc
DISP_CC_MDSS_MDP_CLK>,
+ <&dispcc
DISP_CC_MDSS_VSYNC_CLK>,
+ <&dispcc DISP_CC_MDSS_AHB_CLK>;
+ assigned-clock-rates = <3>,
+ <1920>,
+ <1920>;
+ operating-points-v2 = <&mdp_opp_table>;
+ power-domains = <&rpmhpd SC7280_CX>;
+
+ interrupt-parent = <&mdss>;
+ interrupts = <0>;
+
+ status = "disabled";
+
+ mdp_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-2 {
+ opp-hz = /bits/ 64 <2>;
+ required-opps =
<&rpmhpd_opp_low_svs>;
+ };
+
+ opp-3 {
+ opp-hz = /bits/ 64 <3>;
+ required-opps =
<&rpmhpd_opp_svs>;
+ };
+
+ opp-38000 {
+ opp-hz = /bits/ 64 <38000>;
+ required-opps =
<&rpmhpd_opp_svs_l1>;
+ };
+
+ opp-50667 {
+ opp-hz = /bits/ 64 <50667>;
+
[PATCH v3 2/3] arm64: dts: qcom: sc7280: Add DSI display nodes
From: Krishna Manikandan
Add DSI controller and PHY nodes for sc7280.
Signed-off-by: Rajeev Nandan
Signed-off-by: Krishna Manikandan
Reviewed-by: Matthias Kaehlcke
Reviewed-by: Stephen Boyd
Signed-off-by: Sankeerth Billakanti
---
Changes in v3:
- Add the dsi_phy clocks (Kuogee Hsieh)
- One clock cell per line (Stephen Boyd)
Changes in v2:
- Drop flags from interrupts (Stephen Boyd)
- Rename dsi-opp-table (Stephen Boyd)
- Rename dsi phy node (Stephen Boyd)
arch/arm64/boot/dts/qcom/sc7280.dtsi | 111 ++-
1 file changed, 109 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index a4536b6..12c4d32 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2691,8 +2691,14 @@
reg = <0 0xaf0 0 0x2>;
clocks = <&rpmhcc RPMH_CXO_CLK>,
<&gcc GCC_DISP_GPLL0_CLK_SRC>,
-<0>, <0>, <0>, <0>, <0>, <0>;
- clock-names = "bi_tcxo", "gcc_disp_gpll0_clk",
+<&dsi_phy 0>,
+<&dsi_phy 1>,
+<0>,
+<0>,
+<0>,
+<0>;
+ clock-names = "bi_tcxo",
+ "gcc_disp_gpll0_clk",
"dsi0_phy_pll_out_byteclk",
"dsi0_phy_pll_out_dsiclk",
"dp_phy_pll_link_clk",
@@ -2768,6 +2774,18 @@
status = "disabled";
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ dpu_intf1_out: endpoint {
+ remote-endpoint =
<&dsi0_in>;
+ };
+ };
+ };
+
mdp_opp_table: opp-table {
compatible = "operating-points-v2";
@@ -2792,6 +2810,95 @@
};
};
};
+
+ dsi0: dsi@ae94000 {
+ compatible = "qcom,mdss-dsi-ctrl";
+ reg = <0 0x0ae94000 0 0x400>;
+ reg-names = "dsi_ctrl";
+
+ interrupt-parent = <&mdss>;
+ interrupts = <4>;
+
+ clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK>,
+<&dispcc DISP_CC_MDSS_BYTE0_INTF_CLK>,
+<&dispcc DISP_CC_MDSS_PCLK0_CLK>,
+<&dispcc DISP_CC_MDSS_ESC0_CLK>,
+<&dispcc DISP_CC_MDSS_AHB_CLK>,
+<&gcc GCC_DISP_HF_AXI_CLK>;
+ clock-names = "byte",
+ "byte_intf",
+ "pixel",
+ "core",
+ "iface",
+ "bus";
+
+ operating-points-v2 = <&dsi_opp_table>;
+ power-domains = <&rpmhpd SC7280_CX>;
+
+ phys = <&dsi_phy>;
+ phy-names = "dsi";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ dsi0_in: endpoint {
+ remote-endpoint =
<&dpu_intf1_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ dsi0_out: endpoint {
+ };
+ };
+ };
+
+
[PATCH v3 3/3] arm64: dts: qcom: sc7280: add edp display dt nodes
Add edp controller and phy DT nodes for sc7280.
Signed-off-by: Krishna Manikandan
Reviewed-by: Stephen Boyd
Signed-off-by: Sankeerth Billakanti
---
Changes in v3:
- Add one clock cell per line (Stephen Boyd)
- Unit address should match first reg property (Stephen Boyd)
- Remove new line (Stephen Boyd)
- Add the dsi_phy clocks in dispcc (Kuogee Hsieh)
Changes in v2:
- Move regulator definitions to board file (Matthias Kaehlcke)
- Move the gpio definitions to board file (Matthias Kaehlcke)
- Move the pinconf to board file (Matthias Kaehlcke)
- Move status property (Stephen Boyd)
- Drop flags from interrupts (Stephen Boyd)
- Add clock names one per line for readability (Stephen Boyd)
- Rename edp-opp-table (Stephen Boyd)
arch/arm64/boot/dts/qcom/sc7280.dtsi | 107 ++-
1 file changed, 105 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 12c4d32..5ad500e 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2695,8 +2695,8 @@
<&dsi_phy 1>,
<0>,
<0>,
-<0>,
-<0>;
+<&edp_phy 0>,
+<&edp_phy 1>;
clock-names = "bi_tcxo",
"gcc_disp_gpll0_clk",
"dsi0_phy_pll_out_byteclk",
@@ -2784,6 +2784,13 @@
remote-endpoint =
<&dsi0_in>;
};
};
+
+ port@1 {
+ reg = <1>;
+ dpu_intf5_out: endpoint {
+ remote-endpoint =
<&edp_in>;
+ };
+ };
};
mdp_opp_table: opp-table {
@@ -2899,6 +2906,102 @@
status = "disabled";
};
+
+ msm_edp: edp@aea {
+ compatible = "qcom,sc7280-edp";
+
+ reg = <0 0xaea 0 0x200>,
+ <0 0xaea0200 0 0x200>,
+ <0 0xaea0400 0 0xc00>,
+ <0 0xaea1000 0 0x400>;
+
+ interrupt-parent = <&mdss>;
+ interrupts = <14>;
+
+ clocks = <&rpmhcc RPMH_CXO_CLK>,
+<&gcc GCC_EDP_CLKREF_EN>,
+<&dispcc DISP_CC_MDSS_AHB_CLK>,
+<&dispcc DISP_CC_MDSS_EDP_AUX_CLK>,
+<&dispcc DISP_CC_MDSS_EDP_LINK_CLK>,
+<&dispcc
DISP_CC_MDSS_EDP_LINK_INTF_CLK>,
+<&dispcc DISP_CC_MDSS_EDP_PIXEL_CLK>;
+ clock-names = "core_xo",
+ "core_ref",
+ "core_iface",
+ "core_aux",
+ "ctrl_link",
+ "ctrl_link_iface",
+ "stream_pixel";
+ #clock-cells = <1>;
+ assigned-clocks = <&dispcc
DISP_CC_MDSS_EDP_LINK_CLK_SRC>,
+ <&dispcc
DISP_CC_MDSS_EDP_PIXEL_CLK_SRC>;
+ assigned-clock-parents = <&edp_phy 0>,
<&edp_phy 1>;
+
+ phys = <&edp_phy>;
+ phy-names = "dp";
+
+ operating-points-v2 = <&edp_opp_table>;
+ power-domains = <&rpmhpd SC7280_CX>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ edp_in: endpoint {
+ remote-endpoint =
<&dpu_intf5_out>;
+
Re: [PATCH 02/29] drm/i915/gvt: integrate into the main Makefile
On Tue, 02 Nov 2021, Christoph Hellwig wrote: > Remove the separately included Makefile and just use the relative > reference from the main i915 Makefile as for source files in other > subdirectories. > > Signed-off-by: Christoph Hellwig Acked-by: Jani Nikula > --- > drivers/gpu/drm/i915/Makefile | 29 - > drivers/gpu/drm/i915/gvt/Makefile | 9 - > drivers/gpu/drm/i915/gvt/trace.h | 2 +- > 3 files changed, 25 insertions(+), 15 deletions(-) > delete mode 100644 drivers/gpu/drm/i915/gvt/Makefile > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index 335ba9f43d8f7..63523032eea26 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -295,11 +295,30 @@ i915-$(CONFIG_DRM_I915_SELFTEST) += \ > > # virtual gpu code > i915-y += i915_vgpu.o > - > -ifeq ($(CONFIG_DRM_I915_GVT),y) > -i915-y += intel_gvt.o > -include $(src)/gvt/Makefile > -endif > +i915-$(CONFIG_DRM_I915_GVT) += \ > + intel_gvt.o \ > + gvt/gvt.o \ > + gvt/aperture_gm.o \ > + gvt/handlers.o \ > + gvt/vgpu.o \ > + gvt/trace_points.o \ > + gvt/firmware.o \ > + gvt/interrupt.o \ > + gvt/gtt.o \ > + gvt/cfg_space.o \ > + gvt/opregion.o \ > + gvt/mmio.o \ > + gvt/display.o \ > + gvt/edid.o \ > + gvt/execlist.o \ > + gvt/scheduler.o \ > + gvt/sched_policy.o \ > + gvt/mmio_context.o \ > + gvt/cmd_parser.o \ > + gvt/debugfs.o \ > + gvt/fb_decoder.o \ > + gvt/dmabuf.o \ > + gvt/page_track.o > > obj-$(CONFIG_DRM_I915) += i915.o > obj-$(CONFIG_DRM_I915_GVT_KVMGT) += gvt/kvmgt.o > diff --git a/drivers/gpu/drm/i915/gvt/Makefile > b/drivers/gpu/drm/i915/gvt/Makefile > deleted file mode 100644 > index ea8324abc784a..0 > --- a/drivers/gpu/drm/i915/gvt/Makefile > +++ /dev/null > @@ -1,9 +0,0 @@ > -# SPDX-License-Identifier: GPL-2.0 > -GVT_DIR := gvt > -GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o > firmware.o \ > - interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \ > - execlist.o scheduler.o sched_policy.o mmio_context.o cmd_parser.o > debugfs.o \ > - fb_decoder.o dmabuf.o page_track.o > - > -ccflags-y+= -I $(srctree)/$(src) -I > $(srctree)/$(src)/$(GVT_DIR)/ > -i915-y += $(addprefix $(GVT_DIR)/, > $(GVT_SOURCE)) > diff --git a/drivers/gpu/drm/i915/gvt/trace.h > b/drivers/gpu/drm/i915/gvt/trace.h > index 6d787750d279f..348f57f8301db 100644 > --- a/drivers/gpu/drm/i915/gvt/trace.h > +++ b/drivers/gpu/drm/i915/gvt/trace.h > @@ -379,5 +379,5 @@ TRACE_EVENT(render_mmio, > #undef TRACE_INCLUDE_PATH > #define TRACE_INCLUDE_PATH . > #undef TRACE_INCLUDE_FILE > -#define TRACE_INCLUDE_FILE trace > +#define TRACE_INCLUDE_FILE gvt/trace > #include -- Jani Nikula, Intel Open Source Graphics Center
[PATCH 00/13] drm: Add generic helpers for HDMI scrambling
Hi, This is a follow-up of the work to support the interactions between the hotplug and the scrambling support for vc4: https://lore.kernel.org/dri-devel/20210507150515.257424-11-max...@cerno.tech/ https://lore.kernel.org/dri-devel/20211025152903.1088803-10-max...@cerno.tech/ Ville feedback was that the same discussion happened some time ago for i915, and resulted in a function to do an full disable/enable cycle on reconnection to avoid breaking the HDMI 2.0 spec. This series improves the current scrambling support by adding generic helpers for usual scrambling-related operations, and builds upon them to provide a generic alternative to intel_hdmi_reset_link. Let me know what you think, Maxime Maxime Ripard (13): drm/connector: Add define for HDMI 1.4 Maximum Pixel Rate drm/connector: Add helper to check if a mode requires scrambling drm/atomic: Add HDMI scrambler state helper drm/atomic: Add HDMI reset link helper drm/scdc: Document hotplug gotchas drm/vc4: hdmi: Remove unused argument in vc4_hdmi_supports_scrambling drm/vc4: hdmi: Remove mutex in detect drm/vc4: hdmi: Remove HDMI flag from encoder drm/vc4: hdmi: Simplify the hotplug handling drm/vc4: hdmi: Simplify the connector state retrieval drm/vc4: hdmi: Switch to detect_ctx drm/vc4: hdmi: Leverage new SCDC atomic_check drm/vc4: hdmi: Reset link on hotplug drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 +- drivers/gpu/drm/drm_atomic_helper.c| 109 ++ drivers/gpu/drm/drm_atomic_state_helper.c | 58 ++ drivers/gpu/drm/drm_edid.c | 2 +- drivers/gpu/drm/drm_scdc_helper.c | 13 ++ drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +- drivers/gpu/drm/meson/meson_dw_hdmi.c | 4 +- drivers/gpu/drm/radeon/radeon_encoders.c | 2 +- drivers/gpu/drm/sti/sti_hdmi_tx3g4c28phy.c | 2 +- drivers/gpu/drm/tegra/sor.c| 10 +- drivers/gpu/drm/vc4/vc4_hdmi.c | 232 + drivers/gpu/drm/vc4/vc4_hdmi.h | 17 +- include/drm/drm_atomic_helper.h| 3 + include/drm/drm_atomic_state_helper.h | 3 + include/drm/drm_connector.h| 27 +++ include/drm/drm_modes.h| 15 ++ 16 files changed, 342 insertions(+), 161 deletions(-) -- 2.32.0
[PATCH 02/13] drm/connector: Add helper to check if a mode requires scrambling
Most drivers supporting the HDMI scrambling seem to have the HDMI 1.4
maximum frequency open-coded, and a function to test whether a display
mode is above that threshold to control whether or not scrambling should
be enabled.
Let's create a common define and helper for drivers to reuse.
Cc: Emma Anholt
Cc: Jonathan Hunter
Cc: linux-te...@vger.kernel.org
Cc: Thierry Reding
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/tegra/sor.c| 4 ++--
drivers/gpu/drm/vc4/vc4_hdmi.c | 21 +
include/drm/drm_modes.h| 15 +++
3 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 99a2d627bfeb..390bd04b0d22 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -2192,11 +2192,11 @@ static void tegra_sor_hdmi_scdc_work(struct work_struct
*work)
static void tegra_sor_hdmi_scdc_start(struct tegra_sor *sor)
{
struct drm_scdc *scdc = &sor->output.connector.display_info.hdmi.scdc;
- struct drm_display_mode *mode;
+ const struct drm_display_mode *mode;
mode = &sor->output.encoder.crtc->state->adjusted_mode;
- if (mode->clock >= DRM_HDMI_14_MAX_TMDS_CLK_KHZ && scdc->supported) {
+ if (drm_mode_hdmi_requires_scrambling(mode) && scdc->supported) {
schedule_delayed_work(&sor->scdc, msecs_to_jiffies(5000));
tegra_sor_hdmi_scdc_enable(sor);
sor->scdc_enabled = true;
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index fc7247cc1022..fa76ae2c94e4 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -99,11 +99,6 @@
#define HDMI_14_MAX_TMDS_CLK (DRM_HDMI_14_MAX_TMDS_CLK_KHZ * 1000)
-static bool vc4_hdmi_mode_needs_scrambling(const struct drm_display_mode *mode)
-{
- return mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ;
-}
-
static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
{
struct drm_info_node *node = (struct drm_info_node *)m->private;
@@ -260,10 +255,10 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
if (vc4_hdmi->disable_4kp60) {
struct drm_device *drm = connector->dev;
- struct drm_display_mode *mode;
+ const struct drm_display_mode *mode;
list_for_each_entry(mode, &connector->probed_modes, head) {
- if (vc4_hdmi_mode_needs_scrambling(mode)) {
+ if (drm_mode_hdmi_requires_scrambling(mode)) {
drm_warn_once(drm, "The core clock cannot reach
frequencies high enough to support 4k @ 60Hz.");
drm_warn_once(drm, "Please change your
config.txt file to add hdmi_enable_4kp60.");
}
@@ -574,7 +569,7 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder
*encoder)
}
static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder,
-struct drm_display_mode *mode)
+const struct drm_display_mode *mode)
{
struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
@@ -597,7 +592,7 @@ static bool vc4_hdmi_supports_scrambling(struct drm_encoder
*encoder,
static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder)
{
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
- struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode;
+ const struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode;
unsigned long flags;
lockdep_assert_held(&vc4_hdmi->mutex);
@@ -605,7 +600,7 @@ static void vc4_hdmi_enable_scrambling(struct drm_encoder
*encoder)
if (!vc4_hdmi_supports_scrambling(encoder, mode))
return;
- if (!vc4_hdmi_mode_needs_scrambling(mode))
+ if (!drm_mode_hdmi_requires_scrambling(mode))
return;
drm_scdc_set_high_tmds_clock_ratio(vc4_hdmi->ddc, true);
@@ -1283,7 +1278,8 @@ static int vc4_hdmi_encoder_atomic_check(struct
drm_encoder *encoder,
if (pixel_rate > vc4_hdmi->variant->max_pixel_clock)
return -EINVAL;
- if (vc4_hdmi->disable_4kp60 && (pixel_rate > HDMI_14_MAX_TMDS_CLK))
+ if (vc4_hdmi->disable_4kp60 &&
+ drm_mode_hdmi_requires_scrambling(mode))
return -EINVAL;
vc4_state->pixel_rate = pixel_rate;
@@ -1305,7 +1301,8 @@ vc4_hdmi_encoder_mode_valid(struct drm_encoder *encoder,
if ((mode->clock * 1000) > vc4_hdmi->variant->max_pixel_clock)
return MODE_CLOCK_HIGH;
- if (vc4_hdmi->disable_4kp60 && vc4_hdmi_mode_needs_scrambling(mode))
+ if (vc4_hdmi->disable_4kp60 &&
+ drm_mode_hdmi_requires_scrambling(mode))
return MODE_CLOCK_HIGH;
return M
[PATCH 01/13] drm/connector: Add define for HDMI 1.4 Maximum Pixel Rate
A lot of drivers open-code the HDMI 1.4 maximum pixel rate in their
driver to test whether the resolutions are supported or if the
scrambling needs to be enabled.
Let's create a common define for everyone to use it.
Cc: Alex Deucher
Cc: amd-...@lists.freedesktop.org
Cc: Andrzej Hajda
Cc: Benjamin Gaignard
Cc: "Christian König"
Cc: Emma Anholt
Cc: intel-...@lists.freedesktop.org
Cc: Jani Nikula
Cc: Jernej Skrabec
Cc: Jerome Brunet
Cc: Jonas Karlman
Cc: Jonathan Hunter
Cc: Joonas Lahtinen
Cc: Kevin Hilman
Cc: Laurent Pinchart
Cc: linux-amlo...@lists.infradead.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-te...@vger.kernel.org
Cc: Martin Blumenstingl
Cc: Neil Armstrong
Cc: "Pan, Xinhui"
Cc: Robert Foss
Cc: Rodrigo Vivi
Cc: Thierry Reding
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 ++--
drivers/gpu/drm/drm_edid.c | 2 +-
drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
drivers/gpu/drm/meson/meson_dw_hdmi.c | 4 ++--
drivers/gpu/drm/radeon/radeon_encoders.c | 2 +-
drivers/gpu/drm/sti/sti_hdmi_tx3g4c28phy.c | 2 +-
drivers/gpu/drm/tegra/sor.c| 8
drivers/gpu/drm/vc4/vc4_hdmi.c | 4 ++--
include/drm/drm_connector.h| 2 ++
9 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 62ae63565d3a..3a58db357be0 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -46,7 +46,7 @@
/* DW-HDMI Controller >= 0x200a are at least compliant with SCDC version 1 */
#define SCDC_MIN_SOURCE_VERSION0x1
-#define HDMI14_MAX_TMDSCLK 34000
+#define HDMI14_MAX_TMDSCLK (DRM_HDMI_14_MAX_TMDS_CLK_KHZ * 1000)
enum hdmi_datamap {
RGB444_8B = 0x01,
@@ -1264,7 +1264,7 @@ static bool dw_hdmi_support_scdc(struct dw_hdmi *hdmi,
* for low rates is not supported either
*/
if (!display->hdmi.scdc.scrambling.low_rates &&
- display->max_tmds_clock <= 34)
+ display->max_tmds_clock <= DRM_HDMI_14_MAX_TMDS_CLK_KHZ)
return false;
return true;
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 7aa2a56a71c8..ec8fb2d098ae 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4966,7 +4966,7 @@ static void drm_parse_hdmi_forum_vsdb(struct
drm_connector *connector,
u32 max_tmds_clock = hf_vsdb[5] * 5000;
struct drm_scdc *scdc = &hdmi->scdc;
- if (max_tmds_clock > 34) {
+ if (max_tmds_clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ) {
display->max_tmds_clock = max_tmds_clock;
DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n",
display->max_tmds_clock);
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index d2e61f6c6e08..0666203d52b7 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2226,7 +2226,7 @@ int intel_hdmi_compute_config(struct intel_encoder
*encoder,
if (scdc->scrambling.low_rates)
pipe_config->hdmi_scrambling = true;
- if (pipe_config->port_clock > 34) {
+ if (pipe_config->port_clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ) {
pipe_config->hdmi_scrambling = true;
pipe_config->hdmi_high_tmds_clock_ratio = true;
}
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c
b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 0afbd1e70bfc..8078667aea0e 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -434,7 +434,7 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void
*data,
readl_relaxed(priv->io_base + _REG(VPU_HDMI_SETTING));
DRM_DEBUG_DRIVER("\"%s\" div%d\n", mode->name,
-mode->clock > 34 ? 40 : 10);
+mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ ? 40 : 10);
/* Enable clocks */
regmap_update_bits(priv->hhi, HHI_HDMI_CLK_CNTL, 0x, 0x100);
@@ -457,7 +457,7 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void
*data,
dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_BIST_CNTL, BIT(12));
/* TMDS pattern setup */
- if (mode->clock > 34 &&
+ if (mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ &&
dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_YUV8_1X24) {
dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_01,
0);
diff --git a/drivers/gpu/drm/radeon/radeon_encoders.c
b/drivers/gpu/drm/radeon/radeon_encoders.c
index 46549d5179ee..ddd8100e699f 100644
--- a/drivers/gpu/drm/radeon/radeon_encoders.c
+++ b/drivers/gpu/drm/radeo
[PATCH 03/13] drm/atomic: Add HDMI scrambler state helper
All the drivers that implement the HDMI scrambling setup (dw-hdmi, i915,
tegra, vc4) duplicate the same logic to see if the TMDS ratio or the
scrambling state needs to be modified depending on the current connector
state and CRTC mode.
Since it's basically the same logic everywhere, let's put these two
informations in the connector state, and filled by a atomic_check helper
so that drivers can just use it.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_atomic_state_helper.c | 58 +++
include/drm/drm_atomic_state_helper.h | 3 ++
include/drm/drm_connector.h | 25 ++
3 files changed, 86 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
b/drivers/gpu/drm/drm_atomic_state_helper.c
index ddcf5c2c8e6a..93f40f2975c3 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -454,6 +454,64 @@ void drm_atomic_helper_connector_tv_reset(struct
drm_connector *connector)
}
EXPORT_SYMBOL(drm_atomic_helper_connector_tv_reset);
+/**
+ * drm_atomic_helper_connector_hdmi_check - Checks the state of an HDMI
connector
+ * @connector: DRM connector
+ * @state: DRM atomic state to check
+ *
+ * Checks that an HDMI connector state is sane, and sets the various
+ * HDMI-specific flags in drm_connector_state related to HDMI support.
+ *
+ * Returns:
+ * 0 on success, a negative error code otherwise.
+ */
+int drm_atomic_helper_connector_hdmi_check(struct drm_connector *connector,
+ struct drm_atomic_state *state)
+{
+ struct drm_connector_state *conn_state =
drm_atomic_get_new_connector_state(state,
+
connector);
+ struct drm_display_info *info = &connector->display_info;
+ const struct drm_display_mode *mode;
+ struct drm_crtc_state *crtc_state;
+ struct drm_crtc *crtc;
+ bool required;
+
+ if (!conn_state)
+ return -EINVAL;
+
+ crtc = conn_state->crtc;
+ if (!crtc)
+ return -EINVAL;
+
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state))
+ return PTR_ERR(crtc_state);
+
+ mode = &crtc_state->mode;
+ crtc_state->connectors_changed = true;
+ conn_state->hdmi_needs_scrambling = false;
+ conn_state->hdmi_needs_high_tmds_ratio = false;
+
+ if (!info->is_hdmi)
+ return 0;
+
+ if (!info->hdmi.scdc.supported)
+ return 0;
+
+ required = drm_mode_hdmi_requires_scrambling(mode);
+ if (required && !info->hdmi.scdc.scrambling.supported)
+ return -EINVAL;
+
+ if (info->hdmi.scdc.scrambling.low_rates || required)
+ conn_state->hdmi_needs_scrambling = true;
+
+ if (required)
+ conn_state->hdmi_needs_high_tmds_ratio = true;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_atomic_helper_connector_hdmi_check);
+
/**
* __drm_atomic_helper_connector_duplicate_state - copy atomic connector state
* @connector: connector object
diff --git a/include/drm/drm_atomic_state_helper.h
b/include/drm/drm_atomic_state_helper.h
index 3f8f1d627f7c..3d3d1ff355f4 100644
--- a/include/drm/drm_atomic_state_helper.h
+++ b/include/drm/drm_atomic_state_helper.h
@@ -26,6 +26,7 @@
#include
+struct drm_atomic_state;
struct drm_bridge;
struct drm_bridge_state;
struct drm_crtc;
@@ -71,6 +72,8 @@ void __drm_atomic_helper_connector_reset(struct drm_connector
*connector,
struct drm_connector_state
*conn_state);
void drm_atomic_helper_connector_reset(struct drm_connector *connector);
void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector);
+int drm_atomic_helper_connector_hdmi_check(struct drm_connector *connector,
+ struct drm_atomic_state *state);
void
__drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector,
struct drm_connector_state *state);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 030636635af1..78d3d6c78fcb 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -832,6 +832,31 @@ struct drm_connector_state {
* DRM blob property for HDR output metadata
*/
struct drm_property_blob *hdr_output_metadata;
+
+ /**
+* @hdmi_needs_scrambling:
+*
+* Only relevant for HDMI sink. Tracks whether the scrambling
+* should be turned on for the current sink and mode.
+*
+* Drivers needing this should use
+* drm_atomic_helper_connector_hdmi_check() and use the value
+* set here to enable or disable their scrambler.
+*/
+ bool hdmi_needs_scrambling;
+
+ /**
+* @hdmi_needs_high_tmds_ratio:
+*
+* Only relevant for
[PATCH 08/13] drm/vc4: hdmi: Remove HDMI flag from encoder
The hdmi_monitor flag in the vc4_hdmi_encoder structure is redundant
with the is_hdmi flag in the drm_display_info structure.
Let's convert all the users.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 16 ++--
drivers/gpu/drm/vc4/vc4_hdmi.h | 1 -
2 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 826ca7aaf8d7..288c2bfbf88b 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -217,7 +217,6 @@ vc4_hdmi_connector_detect(struct drm_connector *connector,
bool force)
if (edid) {
cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap,
edid);
- vc4_hdmi->encoder.hdmi_monitor =
drm_detect_hdmi_monitor(edid);
kfree(edid);
}
}
@@ -241,7 +240,6 @@ static void vc4_hdmi_connector_destroy(struct drm_connector
*connector)
static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
{
struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
- struct vc4_hdmi_encoder *vc4_encoder = &vc4_hdmi->encoder;
int ret = 0;
struct edid *edid;
@@ -261,8 +259,6 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
if (!edid)
return -ENODEV;
- vc4_encoder->hdmi_monitor = drm_detect_hdmi_monitor(edid);
-
drm_connector_update_edid_property(connector, edid);
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
@@ -581,13 +577,12 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder
*encoder)
static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder)
{
- struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
struct drm_display_info *display = &vc4_hdmi->connector.display_info;
lockdep_assert_held(&vc4_hdmi->mutex);
- if (!vc4_encoder->hdmi_monitor)
+ if (!display->is_hdmi)
return false;
if (!display->hdmi.scdc.supported ||
@@ -1120,11 +1115,12 @@ static void vc4_hdmi_encoder_pre_crtc_enable(struct
drm_encoder *encoder,
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode;
struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
+ struct drm_display_info *display = &vc4_hdmi->connector.display_info;
unsigned long flags;
mutex_lock(&vc4_hdmi->mutex);
- if (vc4_encoder->hdmi_monitor &&
+ if (display->is_hdmi &&
drm_default_rgb_quant_range(mode) ==
HDMI_QUANTIZATION_RANGE_LIMITED) {
if (vc4_hdmi->variant->csc_setup)
vc4_hdmi->variant->csc_setup(vc4_hdmi, true);
@@ -1149,7 +1145,7 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct
drm_encoder *encoder,
{
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode;
- struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
+ struct drm_display_info *display = &vc4_hdmi->connector.display_info;
bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC;
bool vsync_pos = mode->flags & DRM_MODE_FLAG_PVSYNC;
unsigned long flags;
@@ -1170,7 +1166,7 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct
drm_encoder *encoder,
HDMI_WRITE(HDMI_VID_CTL,
HDMI_READ(HDMI_VID_CTL) & ~VC4_HD_VID_CTL_BLANKPIX);
- if (vc4_encoder->hdmi_monitor) {
+ if (display->is_hdmi) {
HDMI_WRITE(HDMI_SCHEDULER_CONTROL,
HDMI_READ(HDMI_SCHEDULER_CONTROL) |
VC4_HDMI_SCHEDULER_CONTROL_MODE_HDMI);
@@ -1197,7 +1193,7 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct
drm_encoder *encoder,
"!VC4_HDMI_SCHEDULER_CONTROL_HDMI_ACTIVE\n");
}
- if (vc4_encoder->hdmi_monitor) {
+ if (display->is_hdmi) {
spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
WARN_ON(!(HDMI_READ(HDMI_SCHEDULER_CONTROL) &
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 32b0aa447717..460112d68948 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -11,7 +11,6 @@
/* VC4 HDMI encoder KMS struct */
struct vc4_hdmi_encoder {
struct vc4_encoder base;
- bool hdmi_monitor;
bool limited_rgb_range;
};
--
2.32.0
[PATCH 05/13] drm/scdc: Document hotplug gotchas
There's some interactions between the SCDC setup and the disconnection / reconnection of displays. Let's document it and a solution. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_scdc_helper.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/gpu/drm/drm_scdc_helper.c b/drivers/gpu/drm/drm_scdc_helper.c index 48a382464d54..033a9e407acb 100644 --- a/drivers/gpu/drm/drm_scdc_helper.c +++ b/drivers/gpu/drm/drm_scdc_helper.c @@ -34,6 +34,19 @@ * HDMI 2.0 specification. It is a point-to-point protocol that allows the * HDMI source and HDMI sink to exchange data. The same I2C interface that * is used to access EDID serves as the transport mechanism for SCDC. + * + * Note: The SCDC status is going to be lost when the display is + * disconnected. This can happen physically when the user disconnects + * the cable, but also when a display is switched on (such as waking up + * a TV). + * + * This is further complicated by the fact that, upon a disconnection / + * reconnection, KMS won't change the mode on its own. This means that + * one can't just rely on setting the SCDC status on enable, but also + * has to track the connector status changes using interrupts and + * restore the SCDC status. The typical solution for this is to call + * drm_atomic_helper_connector_hdmi_reset_link() in + * drm_connector_helper_funcs.detect_ctx(). */ #define SCDC_I2C_SLAVE_ADDRESS 0x54 -- 2.32.0
[PATCH 07/13] drm/vc4: hdmi: Remove mutex in detect
We recently introduced a new mutex to protect concurrent execution of
ALSA and KMS hooks, and the concurrent access to some of vc4_hdmi
fields.
However, using it in the detect hook was creating a reentrency issue
with CEC code. Indeed, calling cec_s_phys_addr_from_edid from detect
might call the CEC adap_enable hook with the lock held, eventually
resulting in a deadlock.
Since we didn't really need to protect anything at the moment in the CEC
code, the decision was made to ignore the mutex in those CEC hooks,
working around the issue.
However, we can have the same thing happening if we end up triggering a
mode set from the detect callback, for example using
drm_atomic_helper_connector_hdmi_reset_link().
Since we don't really need to protect anything in detect either, let's
just drop the lock in detect, and add it again in CEC.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 89 +-
drivers/gpu/drm/vc4/vc4_hdmi.h | 10 +---
2 files changed, 36 insertions(+), 63 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 49bcb0342cc9..826ca7aaf8d7 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -183,7 +183,16 @@ vc4_hdmi_connector_detect(struct drm_connector *connector,
bool force)
struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
bool connected = false;
- mutex_lock(&vc4_hdmi->mutex);
+ /*
+* NOTE: This function should really take vc4_hdmi->mutex, but
+* doing so results in reentrancy issues since
+* cec_s_phys_addr_from_edid might call .adap_enable, which
+* leads to that funtion being called with our mutex held.
+*
+* Concurrency isn't an issue at the moment since we don't share
+* any state with any of the other frameworks so we can ignore
+* the lock for now.
+*/
WARN_ON(pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev));
@@ -215,13 +224,11 @@ vc4_hdmi_connector_detect(struct drm_connector
*connector, bool force)
vc4_hdmi_enable_scrambling(&vc4_hdmi->encoder.base.base);
pm_runtime_put(&vc4_hdmi->pdev->dev);
- mutex_unlock(&vc4_hdmi->mutex);
return connector_status_connected;
}
cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
pm_runtime_put(&vc4_hdmi->pdev->dev);
- mutex_unlock(&vc4_hdmi->mutex);
return connector_status_disconnected;
}
@@ -238,14 +245,21 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
int ret = 0;
struct edid *edid;
- mutex_lock(&vc4_hdmi->mutex);
+ /*
+* NOTE: This function should really take vc4_hdmi->mutex, but
+* doing so results in reentrancy issues since
+* cec_s_phys_addr_from_edid might call .adap_enable, which
+* leads to that funtion being called with our mutex held.
+*
+* Concurrency isn't an issue at the moment since we don't share
+* any state with any of the other frameworks so we can ignore
+* the lock for now.
+*/
edid = drm_get_edid(connector, vc4_hdmi->ddc);
cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, edid);
- if (!edid) {
- ret = -ENODEV;
- goto out;
- }
+ if (!edid)
+ return -ENODEV;
vc4_encoder->hdmi_monitor = drm_detect_hdmi_monitor(edid);
@@ -265,9 +279,6 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
}
}
-out:
- mutex_unlock(&vc4_hdmi->mutex);
-
return ret;
}
@@ -1996,21 +2007,12 @@ static int vc4_hdmi_cec_enable(struct cec_adapter *adap)
u32 val;
int ret;
- /*
-* NOTE: This function should really take vc4_hdmi->mutex, but doing so
-* results in a reentrancy since cec_s_phys_addr_from_edid() called in
-* .detect or .get_modes might call .adap_enable, which leads to this
-* function being called with that mutex held.
-*
-* Concurrency is not an issue for the moment since we don't share any
-* state with KMS, so we can ignore the lock for now, but we need to
-* keep it in mind if we were to change that assumption.
-*/
-
ret = pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev);
if (ret)
return ret;
+ mutex_lock(&vc4_hdmi->mutex);
+
spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
val = HDMI_READ(HDMI_CEC_CNTRL_5);
@@ -2045,6 +2047,8 @@ static int vc4_hdmi_cec_enable(struct cec_adapter *adap)
spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags);
+ mutex_unlock(&vc4_hdmi->mutex);
+
return 0;
}
@@ -2053,16 +2057,7 @@ static int vc4_hdmi_cec_disable(struct cec_adapter *adap)
struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap);
unsigned long fla
[PATCH 04/13] drm/atomic: Add HDMI reset link helper
During a hotplug cycle (such as a TV going out of suspend, or when the
cable is disconnected and reconnected), the expectation is that the same
state used before the disconnection is reused until the next commit.
However, the HDMI scrambling requires that some flags are set in the
monitor, and those flags are very likely to be reset when the cable has
been disconnected. This will thus result in a blank display, even if the
display pipeline configuration hasn't been modified or is in the exact
same state.
One solution would be to enable the scrambling-related bits again on
reconnection, but the HDMI 2.0 specification (Section 6.1.3.1 -
Scrambling Control) requires that the scrambling enable bit is set
before sending any scrambled video signal. Using that solution would
break that specification expectation.
Thus, we need to do a full modeset on the connector so that we disable
the video signal, enable the scrambling bit, and enable the video signal
again.
The i915 code was doing this already, so let's take its code and
convert it into a generic helper.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_atomic_helper.c | 109
include/drm/drm_atomic_helper.h | 3 +
2 files changed, 112 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 2c0c6ec92820..9f3fcc65e66e 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -38,6 +38,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -3524,3 +3525,111 @@ drm_atomic_helper_bridge_propagate_bus_fmt(struct
drm_bridge *bridge,
return input_fmts;
}
EXPORT_SYMBOL(drm_atomic_helper_bridge_propagate_bus_fmt);
+
+static int modeset_pipe(struct drm_crtc *crtc,
+ struct drm_modeset_acquire_ctx *ctx)
+{
+ struct drm_atomic_state *state;
+ struct drm_crtc_state *crtc_state;
+ int ret;
+
+ state = drm_atomic_state_alloc(crtc->dev);
+ if (!state)
+ return -ENOMEM;
+
+ state->acquire_ctx = ctx;
+
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state)) {
+ ret = PTR_ERR(crtc_state);
+ goto out;
+ }
+
+ crtc_state->connectors_changed = true;
+
+ ret = drm_atomic_commit(state);
+out:
+ drm_atomic_state_put(state);
+
+ return ret;
+}
+
+/**
+ * drm_atomic_helper_connector_hdmi_reset_link() - Resets an HDMI link
+ * @connector: DRM connector we want to reset
+ * @ctx: Lock acquisition context
+ *
+ * This helper is here to restore the HDMI link state after the
+ * connector status has changed, typically when a TV has come out of
+ * suspend or when the HDMI cable has been disconnected and then
+ * reconnected.
+ *
+ * Returns:
+ * 0 on success, a negative error code otherwise.
+ */
+int drm_atomic_helper_connector_hdmi_reset_link(struct drm_connector
*connector,
+ struct drm_modeset_acquire_ctx
*ctx)
+{
+ struct drm_device *drm = connector->dev;
+ struct drm_connector_state *conn_state;
+ struct drm_crtc_state *crtc_state;
+ struct drm_crtc *crtc;
+ u8 config;
+ int ret;
+
+ if (!connector)
+ return 0;
+
+ drm_WARN_ON(drm,
+ (connector->connector_type != DRM_MODE_CONNECTOR_HDMIA) &&
+ (connector->connector_type != DRM_MODE_CONNECTOR_HDMIB));
+
+ ret = drm_modeset_lock(&drm->mode_config.connection_mutex, ctx);
+ if (ret)
+ return ret;
+
+ conn_state = connector->state;
+ crtc = conn_state->crtc;
+ if (!crtc)
+ return 0;
+
+ ret = drm_modeset_lock(&crtc->mutex, ctx);
+ if (ret)
+ return ret;
+
+ crtc_state = crtc->state;
+ if (!crtc_state->active)
+ return 0;
+
+ if (!conn_state->hdmi_needs_high_tmds_ratio &&
+ !conn_state->hdmi_needs_scrambling)
+ return 0;
+
+ if (conn_state->commit &&
+ !try_wait_for_completion(&conn_state->commit->hw_done))
+ return 0;
+
+ ret = drm_scdc_readb(connector->ddc, SCDC_TMDS_CONFIG, &config);
+ if (ret < 0) {
+ drm_err(drm, "Failed to read TMDS config: %d\n", ret);
+ return 0;
+ }
+
+ if (!!(config & SCDC_TMDS_BIT_CLOCK_RATIO_BY_40) ==
+ conn_state->hdmi_needs_high_tmds_ratio &&
+ !!(config & SCDC_SCRAMBLING_ENABLE) ==
+ conn_state->hdmi_needs_scrambling)
+ return 0;
+
+ /*
+* HDMI 2.0 says that one should not send scrambled data
+* prior to configuring the sink scrambling, and that
+* TMDS clock/data transmission should be suspended when
+* changing the TMDS clock rate in the sink. So let's
+* just do a full modeset here, even though some sinks
+* would b
[PATCH 09/13] drm/vc4: hdmi: Simplify the hotplug handling
Our detect callback has a bunch of operations to perform depending on
the current and last status of the connector, such a setting the CEC
physical address or enabling the scrambling again.
This is currently dealt with a bunch of if / else statetements that make
it fairly difficult to read and extend.
Let's move all that logic to a function of its own.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 50 --
1 file changed, 30 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 288c2bfbf88b..4f2f138f93e3 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -177,11 +177,35 @@ static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi
*vc4_hdmi) {}
static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder);
+static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
+ enum drm_connector_status status)
+{
+ struct drm_connector *connector = &vc4_hdmi->connector;
+ struct edid *edid;
+
+ if (status == connector->status)
+ return;
+
+ if (status == connector_status_disconnected) {
+ cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
+ return;
+ }
+
+ edid = drm_get_edid(connector, vc4_hdmi->ddc);
+ if (!edid)
+ return;
+
+ cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, edid);
+ kfree(edid);
+
+ vc4_hdmi_enable_scrambling(&vc4_hdmi->encoder.base.base);
+}
+
static enum drm_connector_status
vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
{
struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
- bool connected = false;
+ enum drm_connector_status status = connector_status_disconnected;
/*
* NOTE: This function should really take vc4_hdmi->mutex, but
@@ -198,7 +222,7 @@ vc4_hdmi_connector_detect(struct drm_connector *connector,
bool force)
if (vc4_hdmi->hpd_gpio) {
if (gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio))
- connected = true;
+ status = connector_status_connected;
} else {
unsigned long flags;
u32 hotplug;
@@ -208,27 +232,13 @@ vc4_hdmi_connector_detect(struct drm_connector
*connector, bool force)
spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags);
if (hotplug & VC4_HDMI_HOTPLUG_CONNECTED)
- connected = true;
+ status = connector_status_connected;
}
- if (connected) {
- if (connector->status != connector_status_connected) {
- struct edid *edid = drm_get_edid(connector,
vc4_hdmi->ddc);
-
- if (edid) {
- cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap,
edid);
- kfree(edid);
- }
- }
-
- vc4_hdmi_enable_scrambling(&vc4_hdmi->encoder.base.base);
- pm_runtime_put(&vc4_hdmi->pdev->dev);
- return connector_status_connected;
- }
-
- cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
+ vc4_hdmi_handle_hotplug(vc4_hdmi, status);
pm_runtime_put(&vc4_hdmi->pdev->dev);
- return connector_status_disconnected;
+
+ return status;
}
static void vc4_hdmi_connector_destroy(struct drm_connector *connector)
--
2.32.0
[PATCH 11/13] drm/vc4: hdmi: Switch to detect_ctx
We'll need the locking context in future patch, so let's convert .detect
to .detect_ctx.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 2d7c34b306c9..82878718e5fc 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -201,8 +201,9 @@ static void vc4_hdmi_handle_hotplug(struct vc4_hdmi
*vc4_hdmi,
vc4_hdmi_enable_scrambling(&vc4_hdmi->encoder.base.base);
}
-static enum drm_connector_status
-vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
+static int vc4_hdmi_connector_detect_ctx(struct drm_connector *connector,
+struct drm_modeset_acquire_ctx *ctx,
+bool force)
{
struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
enum drm_connector_status status = connector_status_disconnected;
@@ -353,7 +354,6 @@ vc4_hdmi_connector_duplicate_state(struct drm_connector
*connector)
}
static const struct drm_connector_funcs vc4_hdmi_connector_funcs = {
- .detect = vc4_hdmi_connector_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
.destroy = vc4_hdmi_connector_destroy,
.reset = vc4_hdmi_connector_reset,
@@ -362,6 +362,7 @@ static const struct drm_connector_funcs
vc4_hdmi_connector_funcs = {
};
static const struct drm_connector_helper_funcs vc4_hdmi_connector_helper_funcs
= {
+ .detect_ctx = vc4_hdmi_connector_detect_ctx,
.get_modes = vc4_hdmi_connector_get_modes,
.atomic_check = vc4_hdmi_connector_atomic_check,
};
--
2.32.0
[PATCH 10/13] drm/vc4: hdmi: Simplify the connector state retrieval
When we have the entire DRM state, retrieving the connector state only
requires the drm_connector pointer. Fortunately for us, we have
allocated it as a part of the vc4_hdmi structure, so we can retrieve get
a pointer by simply accessing our field in that structure.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 21 +++--
1 file changed, 3 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 4f2f138f93e3..2d7c34b306c9 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -996,30 +996,15 @@ static void vc4_hdmi_recenter_fifo(struct vc4_hdmi
*vc4_hdmi)
"VC4_HDMI_FIFO_CTL_RECENTER_DONE");
}
-static struct drm_connector_state *
-vc4_hdmi_encoder_get_connector_state(struct drm_encoder *encoder,
-struct drm_atomic_state *state)
-{
- struct drm_connector_state *conn_state;
- struct drm_connector *connector;
- unsigned int i;
-
- for_each_new_connector_in_state(state, connector, conn_state, i) {
- if (conn_state->best_encoder == encoder)
- return conn_state;
- }
-
- return NULL;
-}
-
static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder,
struct drm_atomic_state *state)
{
+ struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
+ struct drm_connector *connector = &vc4_hdmi->connector;
struct drm_connector_state *conn_state =
- vc4_hdmi_encoder_get_connector_state(encoder, state);
+ drm_atomic_get_new_connector_state(state, connector);
struct vc4_hdmi_connector_state *vc4_conn_state =
conn_state_to_vc4_hdmi_conn_state(conn_state);
- struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode;
unsigned long pixel_rate = vc4_conn_state->pixel_rate;
unsigned long bvb_rate, hsm_rate;
--
2.32.0
[PATCH 06/13] drm/vc4: hdmi: Remove unused argument in vc4_hdmi_supports_scrambling
Even though vc4_hdmi_supports_scrambling takes a mode as an argument, it
never uses it. Let's remove it.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index fa76ae2c94e4..49bcb0342cc9 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -568,8 +568,7 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder
*encoder)
vc4_hdmi_set_hdr_infoframe(encoder);
}
-static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder,
-const struct drm_display_mode *mode)
+static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder)
{
struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
@@ -597,7 +596,7 @@ static void vc4_hdmi_enable_scrambling(struct drm_encoder
*encoder)
lockdep_assert_held(&vc4_hdmi->mutex);
- if (!vc4_hdmi_supports_scrambling(encoder, mode))
+ if (!vc4_hdmi_supports_scrambling(encoder))
return;
if (!drm_mode_hdmi_requires_scrambling(mode))
--
2.32.0
[PATCH 13/13] drm/vc4: hdmi: Reset link on hotplug
Enabling the scrambling on reconnection seems to work so far but breaks
the HDMI2.0 specification and has introduced some issues in the past
with i915.
Let's do a mode set on the connector instead to follow the
specification.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index aa6700622797..a1f40548dd48 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -175,9 +175,8 @@ static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi
*vc4_hdmi)
static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi) {}
#endif
-static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder);
-
static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
+ struct drm_modeset_acquire_ctx *ctx,
enum drm_connector_status status)
{
struct drm_connector *connector = &vc4_hdmi->connector;
@@ -198,7 +197,7 @@ static void vc4_hdmi_handle_hotplug(struct vc4_hdmi
*vc4_hdmi,
cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, edid);
kfree(edid);
- vc4_hdmi_enable_scrambling(&vc4_hdmi->encoder.base.base);
+ drm_atomic_helper_connector_hdmi_reset_link(connector, ctx);
}
static int vc4_hdmi_connector_detect_ctx(struct drm_connector *connector,
@@ -236,7 +235,7 @@ static int vc4_hdmi_connector_detect_ctx(struct
drm_connector *connector,
status = connector_status_connected;
}
- vc4_hdmi_handle_hotplug(vc4_hdmi, status);
+ vc4_hdmi_handle_hotplug(vc4_hdmi, ctx, status);
pm_runtime_put(&vc4_hdmi->pdev->dev);
return status;
--
2.32.0
[PATCH 12/13] drm/vc4: hdmi: Leverage new SCDC atomic_check
Now that we have a generic helper to fill the scrambling status, let's
use it.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 32 ++--
drivers/gpu/drm/vc4/vc4_hdmi.h | 6 ++
2 files changed, 16 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 82878718e5fc..aa6700622797 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -297,6 +297,14 @@ static int vc4_hdmi_connector_atomic_check(struct
drm_connector *connector,
struct drm_connector_state *new_state =
drm_atomic_get_new_connector_state(state, connector);
struct drm_crtc *crtc = new_state->crtc;
+ int ret;
+
+ ret = drm_atomic_helper_connector_hdmi_check(connector, state);
+ if (ret)
+ return ret;
+
+ if (new_state->hdmi_needs_scrambling !=
new_state->hdmi_needs_high_tmds_ratio)
+ return -EINVAL;
if (!crtc)
return 0;
@@ -586,37 +594,16 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder
*encoder)
vc4_hdmi_set_hdr_infoframe(encoder);
}
-static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder)
-{
- struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
- struct drm_display_info *display = &vc4_hdmi->connector.display_info;
-
- lockdep_assert_held(&vc4_hdmi->mutex);
-
- if (!display->is_hdmi)
- return false;
-
- if (!display->hdmi.scdc.supported ||
- !display->hdmi.scdc.scrambling.supported)
- return false;
-
- return true;
-}
-
#define SCRAMBLING_POLLING_DELAY_MS1000
static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder)
{
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
- const struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode;
unsigned long flags;
lockdep_assert_held(&vc4_hdmi->mutex);
- if (!vc4_hdmi_supports_scrambling(encoder))
- return;
-
- if (!drm_mode_hdmi_requires_scrambling(mode))
+ if (!vc4_hdmi->scdc_needed)
return;
drm_scdc_set_high_tmds_clock_ratio(vc4_hdmi->ddc, true);
@@ -1228,6 +1215,7 @@ static void vc4_hdmi_encoder_atomic_mode_set(struct
drm_encoder *encoder,
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
mutex_lock(&vc4_hdmi->mutex);
+ vc4_hdmi->scdc_needed = conn_state->hdmi_needs_scrambling;
memcpy(&vc4_hdmi->saved_adjusted_mode,
&crtc_state->adjusted_mode,
sizeof(vc4_hdmi->saved_adjusted_mode));
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 460112d68948..1aabc51ede03 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -206,6 +206,12 @@ struct vc4_hdmi {
* the scrambler on? Protected by @mutex.
*/
bool scdc_enabled;
+
+ /**
+* @scdc_needed: Is the HDMI controller needs to have the
+* scrambling on? Protected by @mutex.
+*/
+ bool scdc_needed;
};
static inline struct vc4_hdmi *
--
2.32.0
Re: [PATCH -V2] drm/sun4i: Grab reference of connector before return connector from sun4i_tcon_get_connector
Hi,
On Tue, Nov 02, 2021 at 04:46:28AM -0400, He Ying wrote:
> From the comments of drm_for_each_connector_iter(), we know
> that "connector is only valid within the list body, if you
> want to use connector after calling drm_connector_list_iter_end()
> then you need to grab your own reference first using
> drm_connector_get()". So fix the wrong use of connector
> according to the comments and then call drm_connector_put()
> after using connector finishes.
>
> Signed-off-by: He Ying
> ---
>
> V2:
> Use proper subject prefix
>
> drivers/gpu/drm/sun4i/sun4i_tcon.c | 18 +-
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index 9f06dec0fc61..24fa6784ee5f 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -47,12 +47,12 @@ static struct drm_connector
> *sun4i_tcon_get_connector(const struct drm_encoder *
> drm_connector_list_iter_begin(encoder->dev, &iter);
> drm_for_each_connector_iter(connector, &iter)
> if (connector->encoder == encoder) {
> - drm_connector_list_iter_end(&iter);
> - return connector;
> + drm_connector_get(connector);
> + break;
> }
> drm_connector_list_iter_end(&iter);
>
> - return NULL;
> + return connector;
Connector might be uninitialized if we don't find one here
> }
>
> static int sun4i_tcon_get_pixel_depth(const struct drm_encoder *encoder)
> @@ -65,6 +65,7 @@ static int sun4i_tcon_get_pixel_depth(const struct
> drm_encoder *encoder)
> return -EINVAL;
>
> info = &connector->display_info;
> + drm_connector_put(connector);
> if (info->num_bus_formats != 1)
We're still accessing connector->display_info here, but it might have been
freed already.
Maxime
signature.asc
Description: PGP signature
Re: [PATCH] drm/sun4i: fix unmet dependency on RESET_CONTROLLER for PHY_SUN6I_MIPI_DPHY
Hi, On Thu, Oct 28, 2021 at 06:13:27PM -0400, Julian Braha wrote: > When PHY_SUN6I_MIPI_DPHY is selected, and RESET_CONTROLLER > is not selected, Kbuild gives the following warning: > > WARNING: unmet direct dependencies detected for PHY_SUN6I_MIPI_DPHY > Depends on [n]: (ARCH_SUNXI [=n] || COMPILE_TEST [=y]) && HAS_IOMEM [=y] && > COMMON_CLK [=y] && RESET_CONTROLLER [=n] > Selected by [y]: > - DRM_SUN6I_DSI [=y] && HAS_IOMEM [=y] && DRM_SUN4I [=y] > > This is because DRM_SUN6I_DSI selects PHY_SUN6I_MIPI_DPHY > without selecting or depending on RESET_CONTROLLER, despite > PHY_SUN6I_MIPI_DPHY depending on RESET_CONTROLLER. > > These unmet dependency bugs were detected by Kismet, > a static analysis tool for Kconfig. Please advise if this > is not the appropriate solution. > > Signed-off-by: Julian Braha > --- > drivers/gpu/drm/sun4i/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig > index 5755f0432e77..3f70a14a3c94 100644 > --- a/drivers/gpu/drm/sun4i/Kconfig > +++ b/drivers/gpu/drm/sun4i/Kconfig > @@ -46,6 +46,7 @@ config DRM_SUN6I_DSI > default MACH_SUN8I > select CRC_CCITT > select DRM_MIPI_DSI > + select RESET_CONTROLLER The indentation is off, can you fix it and resend it? Maxime signature.asc Description: PGP signature
Re: [PATCH 03/29] drm/i915/gvt: remove module refcounting in intel_gvt_{,un}register_hypervisor
On Tue, Nov 02, 2021 at 08:05:35AM +0100, Christoph Hellwig wrote: > THIS_MODULE always is reference when a symbol called by it is used, so > don't bother with the additional reference. heh, these functions are only called from a module init/exit even Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 04/29] drm/i915/gvt: remove enum hypervisor_type
On Tue, Nov 02, 2021 at 08:05:36AM +0100, Christoph Hellwig wrote: > The only supported hypervisor is KVM, so don't bother with dead code > enumerating hypervisors. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gvt.c | 17 +-- > drivers/gpu/drm/i915/gvt/gvt.h | 1 - > drivers/gpu/drm/i915/gvt/hypercall.h | 6 -- > drivers/gpu/drm/i915/gvt/kvmgt.c | 1 - > drivers/gpu/drm/i915/gvt/opregion.c | 150 ++- > 5 files changed, 34 insertions(+), 141 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 05/29] drm/i915/gvt: rename intel_vgpu_ops to intel_vgpu_mdev_ops
On Tue, Nov 02, 2021 at 08:05:37AM +0100, Christoph Hellwig wrote: > Free the intel_vgpu_ops symbol name for something that fits better. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/kvmgt.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH] drm/virtio: delay pinning the pages till first use
On Tue, Nov 2, 2021 at 6:07 AM Gerd Hoffmann wrote: > > On Tue, Nov 02, 2021 at 12:31:39PM +0100, Maksym Wezdecki wrote: > > From: mwezdeck > > > > The idea behind the commit: > > 1. not pin the pages during resource_create ioctl > > 2. pin the pages on the first use during: > > - transfer_*_host ioctl > > - map ioctl > > i.e. basically lazy pinning. Approach looks sane to me. > > > 3. introduce new ioctl for pinning pages on demand > > What is the use case for this ioctl? > In any case this should be a separate patch. Lazy pinning can be a nice optimization that userspace does not necessarily need to know about. This patch however skips pinning for execbuffer ioctl and introduces a new pin ioctl instead. That is a red flag.
Re: [PATCH v4] backlight: lp855x: Switch to atomic PWM API
On Mon, Nov 01, 2021 at 10:19:21AM -0300, Maíra Canal wrote:
> Remove legacy PWM interface (pwm_config, pwm_enable, pwm_disable) and
> replace it for the atomic PWM API.
>
> Signed-off-by: Maíra Canal
> ---
> V1 -> V2: Initialize variable and simply conditional loop
> V2 -> V3: Fix assignment of NULL variable
> V3 -> V4: Replace division for pwm_set_relative_duty_cycle
> ---
> drivers/video/backlight/lp855x_bl.c | 20
> 1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/video/backlight/lp855x_bl.c
> b/drivers/video/backlight/lp855x_bl.c
> index e94932c69f54..bbf24564082a 100644
> --- a/drivers/video/backlight/lp855x_bl.c
> +++ b/drivers/video/backlight/lp855x_bl.c
> @@ -233,9 +233,8 @@ static int lp855x_configure(struct lp855x *lp)
>
> static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
> {
> - unsigned int period = lp->pdata->period_ns;
> - unsigned int duty = br * period / max_br;
> struct pwm_device *pwm;
> + struct pwm_state state;
>
> /* request pwm device with the consumer name */
> if (!lp->pwm) {
> @@ -245,18 +244,15 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br,
> int max_br)
>
> lp->pwm = pwm;
>
> - /*
> - * FIXME: pwm_apply_args() should be removed when switching to
> - * the atomic PWM API.
> - */
> - pwm_apply_args(pwm);
> + pwm_init_state(lp->pwm, &state);
> + state.period = lp->pdata->period_ns;
> }
>
> - pwm_config(lp->pwm, duty, period);
> - if (duty)
> - pwm_enable(lp->pwm);
> - else
> - pwm_disable(lp->pwm);
> + pwm_get_state(lp->pwm, &state);
Should this be:
} else {
pwm_get_state(lp->pwm, &state);
}
As currently written this will clobber the state.period that was set
above.
Daniel.
> + pwm_set_relative_duty_cycle(&state, br, max_br);
> + state.enabled = state.duty_cycle;
> +
> + pwm_apply_state(lp->pwm, &state);
> }
>
> static int lp855x_bl_update_status(struct backlight_device *bl)
> --
> 2.31.1
>
Re: [PATCH 07/29] drm/i915/gvt: remove intel_gvt_ops
On Tue, Nov 02, 2021 at 08:05:39AM +0100, Christoph Hellwig wrote: > Remove these pointless indirect alls by just calling the only instance > of each method directly. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gvt.c | 20 +-- > drivers/gpu/drm/i915/gvt/gvt.h | 24 -- > drivers/gpu/drm/i915/gvt/hypercall.h | 2 +- > drivers/gpu/drm/i915/gvt/kvmgt.c | 37 +++- > drivers/gpu/drm/i915/gvt/mpt.h | 5 ++-- > 5 files changed, 19 insertions(+), 69 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 08/29] drm/i915/gvt: remove the map_gfn_to_mfn and set_trap_area ops
On Tue, Nov 02, 2021 at 08:05:40AM +0100, Christoph Hellwig wrote: > The map_gfn_to_mfn and set_trap_area ops are never defined, so remove > them and clean up code that depends on them in the callers. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/cfg_space.c | 89 ++-- > drivers/gpu/drm/i915/gvt/hypercall.h | 4 -- > drivers/gpu/drm/i915/gvt/mpt.h | 44 -- > 3 files changed, 17 insertions(+), 120 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 09/29] drm/i915/gvt: remove the unused from_virt_to_mfn op
On Tue, Nov 02, 2021 at 08:05:41AM +0100, Christoph Hellwig wrote: > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/hypercall.h | 1 - > drivers/gpu/drm/i915/gvt/kvmgt.c | 6 -- > drivers/gpu/drm/i915/gvt/mpt.h | 12 > 3 files changed, 19 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 10/29] drm/i915/gvt: merge struct kvmgt_vdev into struct intel_vgpu
On Tue, Nov 02, 2021 at 08:05:42AM +0100, Christoph Hellwig wrote: > Move towards having only a single structure for the per-VGPU state. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gvt.h | 31 ++- > drivers/gpu/drm/i915/gvt/hypercall.h | 1 - > drivers/gpu/drm/i915/gvt/kvmgt.c | 288 ++- > drivers/gpu/drm/i915/gvt/mpt.h | 16 -- > drivers/gpu/drm/i915/gvt/vgpu.c | 8 +- > 5 files changed, 128 insertions(+), 216 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 11/29] drm/i915/gvt: merge struct kvmgt_guest_info into strut intel_vgpu
On Tue, Nov 02, 2021 at 08:05:43AM +0100, Christoph Hellwig wrote: > Consolidate the per-VGPU structures into a single one. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gvt.h | 8 +++ > drivers/gpu/drm/i915/gvt/kvmgt.c | 117 --- > 2 files changed, 52 insertions(+), 73 deletions(-) Reviewed-by: Jason Gunthorpe Jason
[PATCH] drm/i915: fixup dma_fence_wait usage
dma_fence_wait expects a boolean for whether it should be interruptible,
not a timeout value.
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
---
drivers/gpu/drm/i915/i915_vma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 90546fa58fc1..8781c4f61952 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -349,7 +349,7 @@ int i915_vma_wait_for_bind(struct i915_vma *vma)
fence = dma_fence_get_rcu_safe(&vma->active.excl.fence);
rcu_read_unlock();
if (fence) {
- err = dma_fence_wait(fence, MAX_SCHEDULE_TIMEOUT);
+ err = dma_fence_wait(fence, true);
dma_fence_put(fence);
}
}
--
2.31.1
Re: [PATCH 12/29] drm/i915/gvt: remove vgpu->handle
On Tue, Nov 02, 2021 at 08:05:44AM +0100, Christoph Hellwig wrote: > Always pass the actual vgpu structure instead of encoding it as a > "handle" and add a bool flag to denote if a VGPU is attached. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gvt.h | 3 +- > drivers/gpu/drm/i915/gvt/hypercall.h | 32 +++ > drivers/gpu/drm/i915/gvt/kvmgt.c | 126 +-- > drivers/gpu/drm/i915/gvt/mpt.h | 20 ++--- > drivers/gpu/drm/i915/gvt/vgpu.c | 6 +- > 5 files changed, 71 insertions(+), 116 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH] drm/i915: fixup dma_fence_wait usage
On 11/2/21 16:50, Matthew Auld wrote:
dma_fence_wait expects a boolean for whether it should be interruptible,
not a timeout value.
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
---
drivers/gpu/drm/i915/i915_vma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 90546fa58fc1..8781c4f61952 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -349,7 +349,7 @@ int i915_vma_wait_for_bind(struct i915_vma *vma)
fence = dma_fence_get_rcu_safe(&vma->active.excl.fence);
rcu_read_unlock();
if (fence) {
- err = dma_fence_wait(fence, MAX_SCHEDULE_TIMEOUT);
+ err = dma_fence_wait(fence, true);
dma_fence_put(fence);
}
}
LGTM. Reviewed-by: Thomas Hellström
Re: [PATCH 13/29] drm/i915/gvt: devirtualize ->{read,write}_gpa
On Tue, Nov 02, 2021 at 08:05:45AM +0100, Christoph Hellwig wrote: > Just call the VFIO functions directly instead of through the method > table. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/cmd_parser.c | 4 +-- > drivers/gpu/drm/i915/gvt/execlist.c | 12 - > drivers/gpu/drm/i915/gvt/gtt.c| 6 ++--- > drivers/gpu/drm/i915/gvt/gvt.h| 37 +++ > drivers/gpu/drm/i915/gvt/hypercall.h | 4 --- > drivers/gpu/drm/i915/gvt/kvmgt.c | 23 - > drivers/gpu/drm/i915/gvt/mmio.c | 4 +-- > drivers/gpu/drm/i915/gvt/mpt.h| 32 --- > drivers/gpu/drm/i915/gvt/opregion.c | 10 +++- > drivers/gpu/drm/i915/gvt/scheduler.c | 37 +-- > 10 files changed, 72 insertions(+), 97 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 14/29] drm/i915/gvt: devirtualize ->{get,put}_vfio_device
On Tue, Nov 02, 2021 at 08:05:46AM +0100, Christoph Hellwig wrote: > Just open code the calls to the VFIO APIs. > > Signed-off-by: Christoph Hellwig > drivers/gpu/drm/i915/gvt/dmabuf.c| 12 ++- > drivers/gpu/drm/i915/gvt/hypercall.h | 2 -- > drivers/gpu/drm/i915/gvt/kvmgt.c | 22 > drivers/gpu/drm/i915/gvt/mpt.h | 30 > 4 files changed, 7 insertions(+), 59 deletions(-) I suspect the use of get/put here is very questionable It only prevents the vfio_device from becoming unregistered, but the main thing a mdev should care about is if it is still beween opne_device() / close_device() - ie the FD is open, there is a SW IOMMU available, and memory pins can be made. Still, not for this patch Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 15/29] drm/i915/gvt: devirtualize ->set_edid and ->set_opregion
On Tue, Nov 02, 2021 at 08:05:47AM +0100, Christoph Hellwig wrote: > Just call the code to setup the opregions and EDID data directly. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gvt.h | 3 +++ > drivers/gpu/drm/i915/gvt/hypercall.h | 3 --- > drivers/gpu/drm/i915/gvt/kvmgt.c | 6 ++ > drivers/gpu/drm/i915/gvt/mpt.h | 32 > drivers/gpu/drm/i915/gvt/vgpu.c | 6 +++--- > 5 files changed, 8 insertions(+), 42 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 16/29] drm/i915/gvt: devirtualize ->detach_vgpu
On Tue, Nov 02, 2021 at 08:05:48AM +0100, Christoph Hellwig wrote: > Just call the function directly. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gvt.h | 1 + > drivers/gpu/drm/i915/gvt/hypercall.h | 1 - > drivers/gpu/drm/i915/gvt/kvmgt.c | 3 +-- > drivers/gpu/drm/i915/gvt/mpt.h | 16 > drivers/gpu/drm/i915/gvt/vgpu.c | 2 +- > 5 files changed, 3 insertions(+), 20 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 17/29] drm/i915/gvt: devirtualize ->inject_msi
On Tue, Nov 02, 2021 at 08:05:49AM +0100, Christoph Hellwig wrote: > Just open code the MSI injection in a single place instead of going > through the method table. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/hypercall.h | 1 - > drivers/gpu/drm/i915/gvt/interrupt.c | 38 +++- > drivers/gpu/drm/i915/gvt/kvmgt.c | 24 -- > drivers/gpu/drm/i915/gvt/mpt.h | 37 --- > 4 files changed, 37 insertions(+), 63 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [Intel-gfx] [PATCH 11/28] drm/i915/pm: Move CONTEXT_VALID_BIT check
On Thu, 21 Oct 2021 at 11:37, Maarten Lankhorst
wrote:
>
> Resetting will clear the CONTEXT_VALID_BIT, so wait until after that to test.
>
AFAIK this seems to be fixing something earlier in the series(maybe
patch 7?) i.e without this patch we seem to trigger the BUG_ON. If so,
this needs to be much earlier in the series?
Also this probably needs some more commentary in the commit message
for whether moving the BUG_ON matters, or if this is potentially
papering over something significant?
> Signed-off-by: Maarten Lankhorst
> Reviewed-by: Niranjana Vishwanathapura
> ---
> drivers/gpu/drm/i915/gt/intel_engine_pm.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> index a1334b48dde7..849fbb229bd3 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> @@ -52,8 +52,6 @@ static int __engine_unpark(struct intel_wakeref *wf)
> /* Discard stale context state from across idling */
> ce = engine->kernel_context;
> if (ce) {
> - GEM_BUG_ON(test_bit(CONTEXT_VALID_BIT, &ce->flags));
> -
> /* Flush all pending HW writes before we touch the context */
> while (unlikely(intel_context_inflight(ce)))
> intel_engine_flush_submission(engine);
> @@ -68,6 +66,9 @@ static int __engine_unpark(struct intel_wakeref *wf)
> ce->timeline->seqno,
> READ_ONCE(*ce->timeline->hwsp_seqno),
> ce->ring->emit);
> +
> + GEM_BUG_ON(test_bit(CONTEXT_VALID_BIT, &ce->flags));
> +
> GEM_BUG_ON(ce->timeline->seqno !=
>READ_ONCE(*ce->timeline->hwsp_seqno));
> }
> --
> 2.33.0
>
Re: [PATCH 18/29] drm/i915/gvt: devirtualize ->is_valid_gfn
On Tue, Nov 02, 2021 at 08:05:50AM +0100, Christoph Hellwig wrote: > Just call the code directly and move towards the callers. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gtt.c | 20 ++-- > drivers/gpu/drm/i915/gvt/hypercall.h | 1 - > drivers/gpu/drm/i915/gvt/kvmgt.c | 17 - > drivers/gpu/drm/i915/gvt/mpt.h | 17 - > 4 files changed, 18 insertions(+), 37 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 19/29] drm/i915/gvt: devirtualize ->gfn_to_mfn
On Tue, Nov 02, 2021 at 08:05:51AM +0100, Christoph Hellwig wrote: > Just open code it in the only caller. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gtt.c | 9 + > drivers/gpu/drm/i915/gvt/hypercall.h | 1 - > drivers/gpu/drm/i915/gvt/kvmgt.c | 16 > drivers/gpu/drm/i915/gvt/mpt.h | 14 -- > 4 files changed, 5 insertions(+), 35 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 20/29] drm/i915/gvt: devirtualize ->{enable,disable}_page_track
On Tue, Nov 02, 2021 at 08:05:52AM +0100, Christoph Hellwig wrote: > Just call the kvmgt functions directly. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gvt.h| 3 +++ > drivers/gpu/drm/i915/gvt/hypercall.h | 2 -- > drivers/gpu/drm/i915/gvt/kvmgt.c | 6 ++ > drivers/gpu/drm/i915/gvt/mpt.h| 28 --- > drivers/gpu/drm/i915/gvt/page_track.c | 8 > 5 files changed, 9 insertions(+), 38 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH v8 02/10] drm/vc4: hdmi: Fix HPD GPIO detection
On Mon, 25 Oct 2021 at 16:29, Maxime Ripard wrote:
>
> Prior to commit 6800234ceee0 ("drm/vc4: hdmi: Convert to gpiod"), in the
> detect hook, if we had an HPD GPIO we would only rely on it and return
> whatever state it was in.
>
> However, that commit changed that by mistake to only consider the case
> where we have a GPIO and it returns a logical high, and would fall back
> to the other methods otherwise.
>
> Since we can read the EDIDs when the HPD signal is low on some displays,
> we changed the detection status from disconnected to connected, and we
> would ignore an HPD pulse.
>
> Fixes: 6800234ceee0 ("drm/vc4: hdmi: Convert to gpiod")
> Signed-off-by: Maxime Ripard
Reviewed-by: Dave Stevenson
> ---
> drivers/gpu/drm/vc4/vc4_hdmi.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 338968275724..dde67b991ae7 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -190,9 +190,9 @@ vc4_hdmi_connector_detect(struct drm_connector
> *connector, bool force)
>
> WARN_ON(pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev));
>
> - if (vc4_hdmi->hpd_gpio &&
> - gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio)) {
> - connected = true;
> + if (vc4_hdmi->hpd_gpio) {
> + if (gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio))
> + connected = true;
> } else {
> unsigned long flags;
> u32 hotplug;
> --
> 2.31.1
>
Re: [PATCH 21/29] drm/i915/gvt: devirtualize ->dma_{,un}map_guest_page
On Tue, Nov 02, 2021 at 08:05:53AM +0100, Christoph Hellwig wrote: > Just call the functions directly. Also remove a pointless wrapper. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/dmabuf.c| 10 ++ > drivers/gpu/drm/i915/gvt/gtt.c | 20 +-- > drivers/gpu/drm/i915/gvt/gvt.h | 4 > drivers/gpu/drm/i915/gvt/hypercall.h | 5 - > drivers/gpu/drm/i915/gvt/kvmgt.c | 6 ++ > drivers/gpu/drm/i915/gvt/mpt.h | 29 > 6 files changed, 17 insertions(+), 57 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 22/29] drm/i915/gvt: devirtualize dma_pin_guest_page
On Tue, Nov 02, 2021 at 08:05:54AM +0100, Christoph Hellwig wrote: > Just call the function directly and remove a pointless wrapper. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/dmabuf.c| 14 +- > drivers/gpu/drm/i915/gvt/gvt.h | 1 + > drivers/gpu/drm/i915/gvt/hypercall.h | 2 -- > drivers/gpu/drm/i915/gvt/kvmgt.c | 4 +--- > drivers/gpu/drm/i915/gvt/mpt.h | 15 --- > 5 files changed, 3 insertions(+), 33 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 23/29] drm/i915/gvt: remove struct intel_gvt_mpt
On Tue, Nov 02, 2021 at 08:05:55AM +0100, Christoph Hellwig wrote: > Just call the initializion and exit functions directly and remove > this abstraction entirely. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gvt.c | 11 - > drivers/gpu/drm/i915/gvt/gvt.h | 12 ++--- > drivers/gpu/drm/i915/gvt/hypercall.h | 50 --- > drivers/gpu/drm/i915/gvt/kvmgt.c | 39 ++- > drivers/gpu/drm/i915/gvt/mpt.h | 74 > 5 files changed, 17 insertions(+), 169 deletions(-) > delete mode 100644 drivers/gpu/drm/i915/gvt/hypercall.h > delete mode 100644 drivers/gpu/drm/i915/gvt/mpt.h Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 24/29] drm/i915/gvt: remove the extra vfio_device refcounting for dmabufs
On Tue, Nov 02, 2021 at 08:05:56AM +0100, Christoph Hellwig wrote: > All the dmabufs are torn down when th VGPU is released, so there is > no need for extra refcounting here. > > Based on an patch from Jason Gunthorpe. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/dmabuf.c | 12 > drivers/gpu/drm/i915/gvt/gvt.h| 1 - > 2 files changed, 13 deletions(-) Oh, here it gets fixed up, yes: Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 25/29] drm/i915/gvt: streamline intel_vgpu_create
On Tue, Nov 02, 2021 at 08:05:57AM +0100, Christoph Hellwig wrote: > Initialize variables at declaration time, avoid pointless gotos and > cater for the fact that intel_gvt_create_vgpu can't return NULL. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/kvmgt.c | 28 +--- > 1 file changed, 9 insertions(+), 19 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 26/29] drm/i915/gvt: pass a struct intel_vgpu to the vfio read/write helpers
On Tue, Nov 02, 2021 at 08:05:58AM +0100, Christoph Hellwig wrote: > Pass the structure we actually care about instead of deriving it from > the mdev_device in the lower level code. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/kvmgt.c | 28 ++-- > 1 file changed, 14 insertions(+), 14 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH v8 03/10] drm/vc4: Make vc4_crtc_get_encoder public
On Mon, 25 Oct 2021 at 16:29, Maxime Ripard wrote:
>
> We'll need that function in vc4_kms to compute the core clock rate
> requirements.
>
> Signed-off-by: Maxime Ripard
Reviewed-by: Dave Stevenson
> ---
> drivers/gpu/drm/vc4/vc4_crtc.c | 8
> drivers/gpu/drm/vc4/vc4_drv.h | 5 +
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index e3ed52d96f42..7cfd4a097847 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -281,10 +281,10 @@ static u32 vc4_crtc_get_fifo_full_level_bits(struct
> vc4_crtc *vc4_crtc,
> * allows drivers to push pixels to more than one encoder from the
> * same CRTC.
> */
> -static struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc,
> - struct drm_atomic_state
> *state,
> - struct drm_connector_state
> *(*get_state)(struct drm_atomic_state *state,
> -
>struct drm_connector *connector))
> +struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc,
> +struct drm_atomic_state *state,
> +struct drm_connector_state
> *(*get_state)(struct drm_atomic_state *state,
> +
> struct drm_connector *connector))
> {
> struct drm_connector *connector;
> struct drm_connector_list_iter conn_iter;
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 4b550ebd9572..f5e678491502 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -544,6 +544,11 @@ vc4_crtc_to_vc4_pv_data(const struct vc4_crtc *crtc)
> return container_of(data, struct vc4_pv_data, base);
> }
>
> +struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc,
> +struct drm_atomic_state *state,
> +struct drm_connector_state
> *(*get_state)(struct drm_atomic_state *state,
> +
> struct drm_connector *connector));
> +
> struct vc4_crtc_state {
> struct drm_crtc_state base;
> /* Dlist area for this CRTC configuration. */
> --
> 2.31.1
>
Re: [PATCH v8 04/10] drm/vc4: crtc: Add encoder to vc4_crtc_config_pv prototype
On Mon, 25 Oct 2021 at 16:29, Maxime Ripard wrote:
>
> vc4_crtc_config_pv() retrieves the encoder again, even though its only
> caller, vc4_crtc_atomic_enable(), already did.
>
> Pass the encoder pointer as an argument instead of going through all the
> connectors to retrieve it again.
>
> Signed-off-by: Maxime Ripard
Reviewed-by: Dave Stevenson
> ---
> drivers/gpu/drm/vc4/vc4_crtc.c | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index 7cfd4a097847..e5c2e29a6f01 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -315,12 +315,11 @@ static void vc4_crtc_pixelvalve_reset(struct drm_crtc
> *crtc)
> CRTC_WRITE(PV_CONTROL, CRTC_READ(PV_CONTROL) | PV_CONTROL_FIFO_CLR);
> }
>
> -static void vc4_crtc_config_pv(struct drm_crtc *crtc, struct
> drm_atomic_state *state)
> +static void vc4_crtc_config_pv(struct drm_crtc *crtc, struct drm_encoder
> *encoder,
> + struct drm_atomic_state *state)
> {
> struct drm_device *dev = crtc->dev;
> struct vc4_dev *vc4 = to_vc4_dev(dev);
> - struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, state,
> -
> drm_atomic_get_new_connector_state);
> struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
> struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
> const struct vc4_pv_data *pv_data = vc4_crtc_to_vc4_pv_data(vc4_crtc);
> @@ -597,7 +596,7 @@ static void vc4_crtc_atomic_enable(struct drm_crtc *crtc,
> if (vc4_encoder->pre_crtc_configure)
> vc4_encoder->pre_crtc_configure(encoder, state);
>
> - vc4_crtc_config_pv(crtc, state);
> + vc4_crtc_config_pv(crtc, encoder, state);
>
> CRTC_WRITE(PV_CONTROL, CRTC_READ(PV_CONTROL) | PV_CONTROL_EN);
>
> --
> 2.31.1
>
Re: [PATCH v8 05/10] drm/vc4: crtc: Rework the encoder retrieval code (again)
On Mon, 25 Oct 2021 at 16:29, Maxime Ripard wrote:
>
> It turns out the encoder retrieval code, in addition to being
> unnecessarily complicated, has a bug when only the planes and crtcs are
> affected by a given atomic commit.
>
> Indeed, in such a case, either drm_atomic_get_old_connector_state or
> drm_atomic_get_new_connector_state will return NULL and thus our encoder
> retrieval code will not match on anything.
>
> We can however simplify the code by using drm_for_each_encoder_mask, the
> drm_crtc_state storing the encoders a given CRTC is connected to
> directly and without relying on any other state.
>
> Signed-off-by: Maxime Ripard
Reviewed-by: Dave Stevenson
> ---
> drivers/gpu/drm/vc4/vc4_crtc.c | 30 +-
> drivers/gpu/drm/vc4/vc4_drv.h | 4 +---
> 2 files changed, 10 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index e5c2e29a6f01..fbc1d4638650 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -282,26 +282,14 @@ static u32 vc4_crtc_get_fifo_full_level_bits(struct
> vc4_crtc *vc4_crtc,
> * same CRTC.
> */
> struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc,
> -struct drm_atomic_state *state,
> -struct drm_connector_state
> *(*get_state)(struct drm_atomic_state *state,
> -
> struct drm_connector *connector))
> +struct drm_crtc_state *state)
> {
> - struct drm_connector *connector;
> - struct drm_connector_list_iter conn_iter;
> + struct drm_encoder *encoder;
>
> - drm_connector_list_iter_begin(crtc->dev, &conn_iter);
> - drm_for_each_connector_iter(connector, &conn_iter) {
> - struct drm_connector_state *conn_state = get_state(state,
> connector);
> + WARN_ON(hweight32(state->encoder_mask) > 1);
>
> - if (!conn_state)
> - continue;
> -
> - if (conn_state->crtc == crtc) {
> - drm_connector_list_iter_end(&conn_iter);
> - return connector->encoder;
> - }
> - }
> - drm_connector_list_iter_end(&conn_iter);
> + drm_for_each_encoder_mask(encoder, crtc->dev, state->encoder_mask)
> + return encoder;
>
> return NULL;
> }
> @@ -550,8 +538,7 @@ static void vc4_crtc_atomic_disable(struct drm_crtc *crtc,
> struct drm_crtc_state *old_state =
> drm_atomic_get_old_crtc_state(state,
>
> crtc);
> struct vc4_crtc_state *old_vc4_state = to_vc4_crtc_state(old_state);
> - struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, state,
> -
> drm_atomic_get_old_connector_state);
> + struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, old_state);
> struct drm_device *dev = crtc->dev;
>
> require_hvs_enabled(dev);
> @@ -578,10 +565,11 @@ static void vc4_crtc_atomic_disable(struct drm_crtc
> *crtc,
> static void vc4_crtc_atomic_enable(struct drm_crtc *crtc,
>struct drm_atomic_state *state)
> {
> + struct drm_crtc_state *new_state =
> drm_atomic_get_new_crtc_state(state,
> +
> crtc);
> struct drm_device *dev = crtc->dev;
> struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
> - struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, state,
> -
> drm_atomic_get_new_connector_state);
> + struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, new_state);
> struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
>
> require_hvs_enabled(dev);
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index f5e678491502..60826aca9e5b 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -545,9 +545,7 @@ vc4_crtc_to_vc4_pv_data(const struct vc4_crtc *crtc)
> }
>
> struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc,
> -struct drm_atomic_state *state,
> -struct drm_connector_state
> *(*get_state)(struct drm_atomic_state *state,
> -
> struct drm_connector *connector));
> +struct drm_crtc_state *state);
>
> struct vc4_crtc_state {
> struct drm_crtc_state base;
> --
> 2.31.1
>
Re: [PATCH v8 06/10] drm/vc4: crtc: Add some logging
On Mon, 25 Oct 2021 at 16:29, Maxime Ripard wrote: > > The encoder retrieval code has been a source of bugs and glitches in the > past and the crtc <-> encoder association been wrong in a number of > different ways. > > Add some logging to quickly spot issues if they occur. > > Signed-off-by: Maxime Ripard Reviewed-by: Dave Stevenson > --- > drivers/gpu/drm/vc4/vc4_crtc.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c > index fbc1d4638650..6decaa12a078 100644 > --- a/drivers/gpu/drm/vc4/vc4_crtc.c > +++ b/drivers/gpu/drm/vc4/vc4_crtc.c > @@ -541,6 +541,9 @@ static void vc4_crtc_atomic_disable(struct drm_crtc *crtc, > struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, old_state); > struct drm_device *dev = crtc->dev; > > + drm_dbg(dev, "Disabling CRTC %s (%u) connected to Encoder %s (%u)", > + crtc->name, crtc->base.id, encoder->name, encoder->base.id); > + > require_hvs_enabled(dev); > > /* Disable vblank irq handling before crtc is disabled. */ > @@ -572,6 +575,9 @@ static void vc4_crtc_atomic_enable(struct drm_crtc *crtc, > struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, new_state); > struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder); > > + drm_dbg(dev, "Enabling CRTC %s (%u) connected to Encoder %s (%u)", > + crtc->name, crtc->base.id, encoder->name, encoder->base.id); > + > require_hvs_enabled(dev); > > /* Enable vblank irq handling before crtc is started otherwise > -- > 2.31.1 >
Re: [PATCH 27/29] drm/i915/gvt: remove kvmgt_guest_{init,exit}
On Tue, Nov 02, 2021 at 08:05:59AM +0100, Christoph Hellwig wrote: > Merge these into their only callers. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/kvmgt.c | 129 ++- > 1 file changed, 60 insertions(+), 69 deletions(-) Reviewed-by: Jason Gunthorpe Jason
[PATCH v3 0/2] drm/i915: Failsafe migration blits
This patch series introduces failsafe migration blits. The reason for this seemingly strange concept is that if the initial clearing or readback of LMEM fails for some reason[1], and we then set up either GPU- or CPU ptes to the allocated LMEM, we can expose old contents from other clients. So after each migration blit to LMEM, attach a dma-fence callback that checks the migration fence error value and if it's an error, performs a memcpy blit, instead. Patch 1 splits out the TTM move code into separate files Patch 2 implements the failsafe blits and related self-tests [1] There are at least two ways we could trigger exposure of uninitialized LMEM assuming the migration blits themselves never trigger a gpu hang. a) A gpu operation preceding a pipelined eviction blit resets and sets the error fence to -EIO, and the error is propagated across the TTM manager to the clear / swapin blit of a newly allocated TTM resource. It aborts and leaves the memory uninitialized. b) Something wedges the GT while a migration blit is submitted. It ends up never executed and TTM can fault user-space cpu-ptes into uninitialized memory. v3: - Style fixes in second patch (Matthew Auld) Thomas Hellström (2): drm/i915/ttm: Reorganize the ttm move code drm/i915/ttm: Failsafe migration blits drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 328 ++- drivers/gpu/drm/i915/gem/i915_gem_ttm.h | 35 ++ drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 520 ++ drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h | 43 ++ .../drm/i915/gem/selftests/i915_gem_migrate.c | 24 +- 6 files changed, 670 insertions(+), 281 deletions(-) create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h -- 2.31.1
[PATCH v3 1/2] drm/i915/ttm: Reorganize the ttm move code
We are about to introduce failsafe- and asynchronous migration and
ttm moves.
This will add complexity and code to the TTM move code so it makes sense
to split it out to a separate file to make the i915 TTM code easer to
digest.
Split the i915 TTM move code out and since we will have to change the name
of the gpu_binds_iomem() and cpu_maps_iomem() functions anyway,
we alter the name of gpu_binds_iomem() to i915_ttm_gtt_binds_lmem() which
is more reflecting what it is used for.
With this we also add some more documentation. Otherwise there should be
no functional change.
Signed-off-by: Thomas Hellström
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/Makefile| 1 +
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 328 +++
drivers/gpu/drm/i915/gem/i915_gem_ttm.h | 35 ++
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 308 +
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h | 38 +++
5 files changed, 430 insertions(+), 280 deletions(-)
create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 467872cca027..7d0d0b814670 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -154,6 +154,7 @@ gem-y += \
gem/i915_gem_throttle.o \
gem/i915_gem_tiling.o \
gem/i915_gem_ttm.o \
+ gem/i915_gem_ttm_move.o \
gem/i915_gem_ttm_pm.o \
gem/i915_gem_userptr.o \
gem/i915_gem_wait.o \
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 6a05369e2705..6369fb9b2455 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -14,13 +14,9 @@
#include "gem/i915_gem_object.h"
#include "gem/i915_gem_region.h"
#include "gem/i915_gem_ttm.h"
+#include "gem/i915_gem_ttm_move.h"
#include "gem/i915_gem_ttm_pm.h"
-
-#include "gt/intel_engine_pm.h"
-#include "gt/intel_gt.h"
-#include "gt/intel_migrate.h"
-
#define I915_TTM_PRIO_PURGE 0
#define I915_TTM_PRIO_NO_PAGES 1
#define I915_TTM_PRIO_HAS_PAGES 2
@@ -108,28 +104,6 @@ static int i915_ttm_err_to_gem(int err)
return err;
}
-static bool gpu_binds_iomem(struct ttm_resource *mem)
-{
- return mem->mem_type != TTM_PL_SYSTEM;
-}
-
-static bool cpu_maps_iomem(struct ttm_resource *mem)
-{
- /* Once / if we support GGTT, this is also false for cached ttm_tts */
- return mem->mem_type != TTM_PL_SYSTEM;
-}
-
-static enum i915_cache_level
-i915_ttm_cache_level(struct drm_i915_private *i915, struct ttm_resource *res,
-struct ttm_tt *ttm)
-{
- return ((HAS_LLC(i915) || HAS_SNOOP(i915)) && !gpu_binds_iomem(res) &&
- ttm->caching == ttm_cached) ? I915_CACHE_LLC :
- I915_CACHE_NONE;
-}
-
-static void i915_ttm_adjust_lru(struct drm_i915_gem_object *obj);
-
static enum ttm_caching
i915_ttm_select_tt_caching(const struct drm_i915_gem_object *obj)
{
@@ -370,23 +344,14 @@ static void i915_ttm_evict_flags(struct ttm_buffer_object
*bo,
*placement = i915_sys_placement;
}
-static int i915_ttm_move_notify(struct ttm_buffer_object *bo)
-{
- struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
- int ret;
-
- ret = i915_gem_object_unbind(obj, I915_GEM_OBJECT_UNBIND_ACTIVE);
- if (ret)
- return ret;
-
- ret = __i915_gem_object_put_pages(obj);
- if (ret)
- return ret;
-
- return 0;
-}
-
-static void i915_ttm_free_cached_io_rsgt(struct drm_i915_gem_object *obj)
+/**
+ * i915_ttm_free_cached_io_rsgt - Free object cached LMEM information
+ * @obj: The GEM object
+ * This function frees any LMEM-related information that is cached on
+ * the object. For example the radix tree for fast page lookup and the
+ * cached refcounted sg-table
+ */
+void i915_ttm_free_cached_io_rsgt(struct drm_i915_gem_object *obj)
{
struct radix_tree_iter iter;
void __rcu **slot;
@@ -403,56 +368,16 @@ static void i915_ttm_free_cached_io_rsgt(struct
drm_i915_gem_object *obj)
obj->ttm.cached_io_rsgt = NULL;
}
-static void
-i915_ttm_adjust_domains_after_move(struct drm_i915_gem_object *obj)
-{
- struct ttm_buffer_object *bo = i915_gem_to_ttm(obj);
-
- if (cpu_maps_iomem(bo->resource) || bo->ttm->caching != ttm_cached) {
- obj->write_domain = I915_GEM_DOMAIN_WC;
- obj->read_domains = I915_GEM_DOMAIN_WC;
- } else {
- obj->write_domain = I915_GEM_DOMAIN_CPU;
- obj->read_domains = I915_GEM_DOMAIN_CPU;
- }
-}
-
-static void i915_ttm_adjust_gem_after_move(struct drm_i915_gem_object *obj)
-{
- struct ttm_buffer_object *bo = i915_gem_to_ttm(obj);
- unsigned int cache_level;
- unsigned int i;
-
- /*
-* If object was moved to an allowable region, update the object
-
