Re: [PATCH v2 1/2] drm/komeda: Adds SMMU support
Hi Liviu,
On Fri, Jun 07, 2019 at 05:05:59PM +0800, Liviu Dudau wrote:
> Hi Lowry,
>
> On Thu, Jun 06, 2019 at 10:53:05AM +0100, Lowry Li (Arm Technology China)
> wrote:
> > From: "Lowry Li (Arm Technology China)"
> >
> > Adds iommu_connect and disconnect for SMMU support, and configures
> > TBU translation once SMMU has been attached to the display device.
> >
> > Signed-off-by: Lowry Li (Arm Technology China)
> > ---
> > .../gpu/drm/arm/display/komeda/d71/d71_component.c | 5 +++
> > drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c | 49
> > ++
> > drivers/gpu/drm/arm/display/komeda/komeda_dev.c| 18
> > drivers/gpu/drm/arm/display/komeda/komeda_dev.h| 7
> > .../drm/arm/display/komeda/komeda_framebuffer.c| 2 +
> > .../drm/arm/display/komeda/komeda_framebuffer.h| 2 +
> > 6 files changed, 83 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > index 4e26a80..4a48dd6 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > @@ -215,6 +215,8 @@ static void d71_layer_update(struct komeda_component *c,
> > malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
> > malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));
> >
> > + if (kfb->is_va)
> > + ctrl |= L_TBU_EN;
> > malidp_write32_mask(reg, BLK_CONTROL, ctrl_mask, ctrl);
> > }
> >
> > @@ -348,6 +350,9 @@ static void d71_wb_layer_update(struct komeda_component
> > *c,
> >fb->pitches[i] & 0x);
> > }
> >
> > + if (kfb->is_va)
> > + ctrl |= LW_TBU_EN;
> > +
> > malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
> > malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));
> > malidp_write32(reg, BLK_INPUT_ID0, to_d71_input_id(>inputs[0]));
> > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> > b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> > index 8e682c7..1b9e734 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> > @@ -517,6 +517,53 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
> > table->n_formats = ARRAY_SIZE(d71_format_caps_table);
> > }
> >
> > +static int d71_connect_iommu(struct komeda_dev *mdev)
> > +{
> > + struct d71_dev *d71 = mdev->chip_data;
> > + u32 __iomem *reg = d71->gcu_addr;
> > + u32 check_bits = (d71->num_pipelines == 2) ?
> > +GCU_STATUS_TCS0 | GCU_STATUS_TCS1 : GCU_STATUS_TCS0;
> > + int i, ret;
> > +
> > + if (!d71->integrates_tbu)
> > + return -1;
> +
> > + malidp_write32_mask(reg, BLK_CONTROL, 0x7, TBU_CONNECT_MODE);
> > +
> > + ret = dp_wait_cond(has_bits(check_bits, malidp_read32(reg, BLK_STATUS)),
> > + 100, 1000, 1000);
> > + if (ret <= 0) {
>
> You don't want to return -ETIMEDOUT if dp_wait_cond() returns zero. Maybe
> follow the same flow as in d71_disconnect_iommu() ?
>
Sorry, it should be changed as in d71_disconnect_iommu(). Thanks a lot
for pointing this out.
> > + DRM_ERROR("connect to TCU timeout!\n");
> > + malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
> > + return -ETIMEDOUT;
> > + }
> > +
> > + for (i = 0; i < d71->num_pipelines; i++)
> > + malidp_write32_mask(d71->pipes[i]->lpu_addr, LPU_TBU_CONTROL,
> > + LPU_TBU_CTRL_TLBPEN, LPU_TBU_CTRL_TLBPEN);
> > + return 0;
> > +}
> > +
> > +static int d71_disconnect_iommu(struct komeda_dev *mdev)
> > +{
> > + struct d71_dev *d71 = mdev->chip_data;
> > + u32 __iomem *reg = d71->gcu_addr;
> > + u32 check_bits = (d71->num_pipelines == 2) ?
> > +GCU_STATUS_TCS0 | GCU_STATUS_TCS1 : GCU_STATUS_TCS0;
> > + int ret;
> > +
> > + malidp_write32_mask(reg, BLK_CONTROL, 0x7, TBU_DISCONNECT_MODE);
> > +
> > + ret = dp_wait_cond(((malidp_read32(reg, BLK_STATUS) & check_bits) == 0),
> > + 100, 1000, 1000);
> > + if (ret < 0) {
> > + DRM_ERROR("disconnect from TCU timeout!\n");
> > + malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
> > + }
> > +
> > + return ret;
> > +}
> > +
> > static struct komeda_dev_funcs d71_chip_funcs = {
> > .init_format_table = d71_init_fmt_tbl,
> > .enum_resources = d71_enum_resources,
> > @@ -527,6 +574,8 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
> > .on_off_vblank = d71_on_off_vblank,
> > .change_opmode = d71_change_opmode,
> > .flush = d71_flush,
> > + .connect_iommu = d71_connect_iommu,
> > + .disconnect_iommu = d71_disconnect_iommu,
> > };
> >
> > struct komeda_dev_funcs *
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
[Bug 110313] [CI][SHARDS] igt@kms_lease@lease-uevent - fail - Failed assertion: igt_lease_change_detected(uevent_monitor, 1)
https://bugs.freedesktop.org/show_bug.cgi?id=110313 Lakshmi changed: What|Removed |Added Priority|high|low --- Comment #6 from Lakshmi --- (In reply to Mika Kahola from comment #5) > It seems that we haven't seen this bug for a while now. Can we close this > one? This issue occurred only once CI_DRM_5855_full (2 months, 1 week old). We can close this issue if this failure doesn't appear in the the next 3 weeks. Dropping the priority to Low as the reproduction rate is very low. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110865] Rx480 consumes 20w more power in idle than under Windows
https://bugs.freedesktop.org/show_bug.cgi?id=110865 Timothy Arceri changed: What|Removed |Added Version|19.1|DRI git Product|Mesa|DRI QA Contact|dri-devel@lists.freedesktop | |.org| Component|Drivers/Gallium/radeonsi|DRM/AMDgpu -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 06/13] drm/lima: drop DRM_AUTH usage from the driver
Looks good for me:
Reviewed-by: Qiang Yu
Regards,
Qiang
On Thu, Jun 6, 2019 at 6:59 PM Emil Velikov wrote:
>
> On Mon, 27 May 2019 at 09:19, Emil Velikov wrote:
> >
> > From: Emil Velikov
> >
> > The authentication can be circumvented, by design, by using the render
> > node.
> >
> > From the driver POV there is no distinction between primary and render
> > nodes, thus we can drop the token.
> >
> > Cc: Qiang Yu
> > Cc: l...@lists.freedesktop.org
> > Cc: David Airlie
> > Cc: Daniel Vetter
> > Signed-off-by: Emil Velikov
> > ---
> > drivers/gpu/drm/lima/lima_drv.c | 14 +++---
> > 1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/lima/lima_drv.c
> > b/drivers/gpu/drm/lima/lima_drv.c
> > index b29c26cd13b2..ae89938c63b1 100644
> > --- a/drivers/gpu/drm/lima/lima_drv.c
> > +++ b/drivers/gpu/drm/lima/lima_drv.c
> > @@ -231,13 +231,13 @@ static void lima_drm_driver_postclose(struct
> > drm_device *dev, struct drm_file *f
> > }
> >
> > static const struct drm_ioctl_desc lima_drm_driver_ioctls[] = {
> > - DRM_IOCTL_DEF_DRV(LIMA_GET_PARAM, lima_ioctl_get_param,
> > DRM_AUTH|DRM_RENDER_ALLOW),
> > - DRM_IOCTL_DEF_DRV(LIMA_GEM_CREATE, lima_ioctl_gem_create,
> > DRM_AUTH|DRM_RENDER_ALLOW),
> > - DRM_IOCTL_DEF_DRV(LIMA_GEM_INFO, lima_ioctl_gem_info,
> > DRM_AUTH|DRM_RENDER_ALLOW),
> > - DRM_IOCTL_DEF_DRV(LIMA_GEM_SUBMIT, lima_ioctl_gem_submit,
> > DRM_AUTH|DRM_RENDER_ALLOW),
> > - DRM_IOCTL_DEF_DRV(LIMA_GEM_WAIT, lima_ioctl_gem_wait,
> > DRM_AUTH|DRM_RENDER_ALLOW),
> > - DRM_IOCTL_DEF_DRV(LIMA_CTX_CREATE, lima_ioctl_ctx_create,
> > DRM_AUTH|DRM_RENDER_ALLOW),
> > - DRM_IOCTL_DEF_DRV(LIMA_CTX_FREE, lima_ioctl_ctx_free,
> > DRM_AUTH|DRM_RENDER_ALLOW),
> > + DRM_IOCTL_DEF_DRV(LIMA_GET_PARAM, lima_ioctl_get_param,
> > DRM_RENDER_ALLOW),
> > + DRM_IOCTL_DEF_DRV(LIMA_GEM_CREATE, lima_ioctl_gem_create,
> > DRM_RENDER_ALLOW),
> > + DRM_IOCTL_DEF_DRV(LIMA_GEM_INFO, lima_ioctl_gem_info,
> > DRM_RENDER_ALLOW),
> > + DRM_IOCTL_DEF_DRV(LIMA_GEM_SUBMIT, lima_ioctl_gem_submit,
> > DRM_RENDER_ALLOW),
> > + DRM_IOCTL_DEF_DRV(LIMA_GEM_WAIT, lima_ioctl_gem_wait,
> > DRM_RENDER_ALLOW),
> > + DRM_IOCTL_DEF_DRV(LIMA_CTX_CREATE, lima_ioctl_ctx_create,
> > DRM_RENDER_ALLOW),
> > + DRM_IOCTL_DEF_DRV(LIMA_CTX_FREE, lima_ioctl_ctx_free,
> > DRM_RENDER_ALLOW),
> > };
> >
> > static const struct file_operations lima_drm_driver_fops = {
> > --
> > 2.21.0
> >
> Humble poke?
>
> Thanks,
> Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/bridge: analogix_dp: Convert to GPIO descriptors
This converts the Analogix display port to use GPIO descriptors
instead of DT-extracted numbers.
Cc: Douglas Anderson
Cc: Sean Paul
Cc: Marek Szyprowski
Cc: Enric Balletbo i Serra
Signed-off-by: Linus Walleij
---
.../drm/bridge/analogix/analogix_dp_core.c| 28 +--
.../drm/bridge/analogix/analogix_dp_core.h| 4 ++-
.../gpu/drm/bridge/analogix/analogix_dp_reg.c | 14 +-
3 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 225f5e5dd69b..64842bbb2f3d 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -18,8 +18,7 @@
#include
#include
#include
-#include
-#include
+#include
#include
#include
@@ -1585,12 +1584,18 @@ analogix_dp_bind(struct device *dev, struct drm_device
*drm_dev,
dp->force_hpd = of_property_read_bool(dev->of_node, "force-hpd");
- dp->hpd_gpio = of_get_named_gpio(dev->of_node, "hpd-gpios", 0);
- if (!gpio_is_valid(dp->hpd_gpio))
- dp->hpd_gpio = of_get_named_gpio(dev->of_node,
-"samsung,hpd-gpio", 0);
+ /* Try two different names */
+ dp->hpd_gpiod = devm_gpiod_get_optional(dev, "hpd", GPIOD_IN);
+ if (!dp->hpd_gpiod)
+ dp->hpd_gpiod = devm_gpiod_get_optional(dev, "samsung,hpd",
+ GPIOD_IN);
+ if (IS_ERR(dp->hpd_gpiod)) {
+ dev_err(dev, "error getting HDP GPIO: %ld\n",
+ PTR_ERR(dp->hpd_gpiod));
+ return ERR_CAST(dp->hpd_gpiod);
+ }
- if (gpio_is_valid(dp->hpd_gpio)) {
+ if (dp->hpd_gpiod) {
/*
* Set up the hotplug GPIO from the device tree as an interrupt.
* Simply specifying a different interrupt in the device tree
@@ -1598,16 +1603,9 @@ analogix_dp_bind(struct device *dev, struct drm_device
*drm_dev,
* using a GPIO. We also need the actual GPIO specifier so
* that we can get the current state of the GPIO.
*/
- ret = devm_gpio_request_one(>dev, dp->hpd_gpio, GPIOF_IN,
- "hpd_gpio");
- if (ret) {
- dev_err(>dev, "failed to get hpd gpio\n");
- return ERR_PTR(ret);
- }
- dp->irq = gpio_to_irq(dp->hpd_gpio);
+ dp->irq = gpiod_to_irq(dp->hpd_gpiod);
irq_flags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
} else {
- dp->hpd_gpio = -ENODEV;
dp->irq = platform_get_irq(pdev, 0);
irq_flags = 0;
}
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index 769255dc6e99..dc65c2bafa63 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -38,6 +38,8 @@
#define DPCD_VOLTAGE_SWING_SET(x) (((x) & 0x3) << 0)
#define DPCD_VOLTAGE_SWING_GET(x) (((x) >> 0) & 0x3)
+struct gpio_desc;
+
enum link_lane_count_type {
LANE_COUNT1 = 1,
LANE_COUNT2 = 2,
@@ -171,7 +173,7 @@ struct analogix_dp_device {
struct link_train link_train;
struct phy *phy;
int dpms_mode;
- int hpd_gpio;
+ struct gpio_desc*hpd_gpiod;
boolforce_hpd;
boolpsr_enable;
boolfast_train_enable;
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index a5f2763d72e4..7aab2d6d710c 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -12,7 +12,7 @@
#include
#include
-#include
+#include
#include
#include
@@ -397,7 +397,7 @@ void analogix_dp_clear_hotplug_interrupts(struct
analogix_dp_device *dp)
{
u32 reg;
- if (gpio_is_valid(dp->hpd_gpio))
+ if (dp->hpd_gpiod)
return;
reg = HOTPLUG_CHG | HPD_LOST | PLUG;
@@ -411,7 +411,7 @@ void analogix_dp_init_hpd(struct analogix_dp_device *dp)
{
u32 reg;
- if (gpio_is_valid(dp->hpd_gpio))
+ if (dp->hpd_gpiod)
return;
analogix_dp_clear_hotplug_interrupts(dp);
@@ -434,8 +434,8 @@ enum dp_irq_type analogix_dp_get_irq_type(struct
analogix_dp_device *dp)
{
u32 reg;
- if (gpio_is_valid(dp->hpd_gpio)) {
- reg = gpio_get_value(dp->hpd_gpio);
+ if (dp->hpd_gpiod) {
+ reg = gpiod_get_value(dp->hpd_gpiod);
if (reg)
return
[PATCH] drm/bridge: analogix-anx78xx: Drop of_gpio.h include
This include is only used for some gpio drivers and consumers that look up GPIO numbers directly from the device tree. This driver does not use it and only needs . Delete the unused include. Cc: Enric Balletbo i Serra Cc: Jose Abreu Signed-off-by: Linus Walleij --- drivers/gpu/drm/bridge/analogix-anx78xx.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c index c09aaf93ae1b..61b5122e5a52 100644 --- a/drivers/gpu/drm/bridge/analogix-anx78xx.c +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c @@ -20,7 +20,6 @@ #include #include #include -#include #include #include #include -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 06/10] drm/amd: drop use of drmP.h from all header files
Drop use of the deprecated drmP.h header file from all amd header files. This makes it a more smooth process to get rid of drmP.h in the .c files. Added include files and forwards as appropriate. Signed-off-by: Sam Ravnborg Cc: Alex Deucher Cc: "Christian König" Cc: "David (ChunMing) Zhou" Cc: David Airlie Cc: Daniel Vetter --- drivers/gpu/drm/amd/amdgpu/amdgpu_sched.h | 5 - drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 2 -- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 5 - 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.h index 2a1a0c734bdd..12299fd95691 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.h @@ -25,7 +25,10 @@ #ifndef __AMDGPU_SCHED_H__ #define __AMDGPU_SCHED_H__ -#include +enum drm_sched_priority; + +struct drm_device; +struct drm_file; enum drm_sched_priority amdgpu_to_sched_priority(int amdgpu_priority); int amdgpu_sched_ioctl(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h index d3ca2424b5fe..77674a7b9616 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h @@ -28,8 +28,6 @@ #include #include -#include - #undef TRACE_SYSTEM #define TRACE_SYSTEM amdgpu #define TRACE_INCLUDE_FILE amdgpu_trace diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h index 978ff14a7d45..2485d8426e5e 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -26,8 +26,11 @@ #ifndef __AMDGPU_DM_H__ #define __AMDGPU_DM_H__ -#include #include +#include +#include +#include +#include /* * This file contains the definition for amdgpu_display_manager -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 08/10] drm/amd: drop use of drmP.h in display/
Drop all uses of drmP.h in drm/amd/display/. Fix fallout. Signed-off-by: Sam Ravnborg Cc: Alex Deucher Cc: "Christian König" Cc: "David (ChunMing) Zhou" Cc: David Airlie Cc: Daniel Vetter --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +++- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 1 - drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c | 2 -- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 1 - drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c | 1 - 5 files changed, 3 insertions(+), 6 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 443b13ec268d..3a723e553a19 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -53,15 +53,17 @@ #include #include #include +#include #include -#include #include #include #include #include #include +#include #include +#include #if defined(CONFIG_DRM_AMD_DC_DCN1_0) #include "ivsrcid/irqsrcs_dcn_1_0.h" diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index e6cd67342df8..97b2c3b16bef 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -28,7 +28,6 @@ #include #include -#include #include #include #include diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c index fd22b4474dbf..1b59d3d42f7b 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c @@ -23,8 +23,6 @@ * */ -#include - #include "dm_services_types.h" #include "dc.h" diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c index 350e7a620d45..b37e8c9653e1 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c @@ -24,7 +24,6 @@ #include #include -#include #include #include #include "dm_services.h" diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c index d915e8c8769b..022da5d45d4d 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c @@ -26,7 +26,6 @@ #include #include -#include #include #include #include "dm_services.h" -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 05/10] drm/amd: drop use of drmP.h in atom.h
Drop use of the deprecated drmP.h header from atom.h Fix fallout in various files. Signed-off-by: Sam Ravnborg Cc: Alex Deucher Cc: "Christian König" Cc: "David (ChunMing) Zhou" Cc: David Airlie Cc: Daniel Vetter --- drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 1 + drivers/gpu/drm/amd/amdgpu/atom.h | 3 ++- drivers/gpu/drm/amd/display/dc/bios/bios_parser.c | 2 ++ drivers/gpu/drm/amd/display/dc/core/dc_link.c | 2 ++ drivers/gpu/drm/amd/display/dc/dce/dce_abm.c | 2 ++ drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c | 3 +++ drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 4 +++- 7 files changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c index 0a4fba196b84..eba42c752bca 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c @@ -24,6 +24,7 @@ */ #include +#include #include #include #include diff --git a/drivers/gpu/drm/amd/amdgpu/atom.h b/drivers/gpu/drm/amd/amdgpu/atom.h index a39170991afe..4205bbe5d8d7 100644 --- a/drivers/gpu/drm/amd/amdgpu/atom.h +++ b/drivers/gpu/drm/amd/amdgpu/atom.h @@ -26,7 +26,8 @@ #define ATOM_H #include -#include + +struct drm_device; #define ATOM_BIOS_MAGIC0xAA55 #define ATOM_ATI_MAGIC_PTR 0x30 diff --git a/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c b/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c index a4c97d32e751..461eef1de124 100644 --- a/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c +++ b/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c @@ -23,6 +23,8 @@ * */ +#include + #include "dm_services.h" #include "atom.h" diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c b/drivers/gpu/drm/amd/display/dc/core/dc_link.c index b37ecc3ede61..bcb20e71b920 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c @@ -23,6 +23,8 @@ * */ +#include + #include "dm_services.h" #include "atom.h" #include "dm_helpers.h" diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_abm.c b/drivers/gpu/drm/amd/display/dc/dce/dce_abm.c index da96229db53a..dbd8cc600127 100644 --- a/drivers/gpu/drm/amd/display/dc/dce/dce_abm.c +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_abm.c @@ -23,6 +23,8 @@ * */ +#include + #include "dce_abm.h" #include "dm_services.h" #include "reg_helper.h" diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c index 101c09b212ad..d09690fca452 100644 --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c @@ -20,6 +20,9 @@ * OTHER DEALINGS IN THE SOFTWARE. * */ + +#include + #include "hwmgr.h" #include "pp_debug.h" #include "ppatomctrl.h" diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c index 92903a4cc4d8..d5d317f17c28 100644 --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c @@ -20,8 +20,10 @@ * OTHER DEALINGS IN THE SOFTWARE. */ -#include "pp_debug.h" #include +#include + +#include "pp_debug.h" #include "amdgpu.h" #include "amdgpu_smu.h" #include "atomfirmware.h" -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 09/10] drm/amd: drop use of drmP.h in amdgpu/amdgpu*
Drop use of drmP.h in all files named amdgpu* in drm/amd/amdgpu/ Fix fallout. Signed-off-by: Sam Ravnborg Cc: Alex Deucher Cc: "Christian König" Cc: "David (ChunMing) Zhou" Cc: David Airlie Cc: Daniel Vetter --- drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_afmt.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 4 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 7 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c| 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 5 - drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c| 14 +++--- drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 4 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 5 - drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_i2c.c | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c| 4 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c| 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_ioc32.c | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 5 - drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 5 - drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c| 5 - drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_test.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_trace_points.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 5 +++-- drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 2 +- 55 files changed, 103 insertions(+), 63 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c index 56f8ca2a3bb4..1e41367ef74e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c @@ -27,7 +27,7 @@ #include #include #include -#include + #include #include "amdgpu.h" #include "amdgpu_pm.h" diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_afmt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_afmt.c index 3889486f71fe..a4d65973bf7c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_afmt.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_afmt.c @@ -25,7 +25,7 @@ */ #include #include -#include + #include #include "amdgpu.h" diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c index aeead072fa79..822049a78e9f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c @@ -22,7 +22,7 @@ #include "amdgpu_amdkfd.h" #include "amd_shared.h" -#include + #include "amdgpu.h" #include "amdgpu_gfx.h" #include diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c index fa09e11a600c..c49d5ae4e29e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c @@ -23,7 +23,7 @@ #include #include #include -#include + #include "amdgpu.h" #include
[PATCH v1 10/10] drm/amd: drop use of drmP.h in remaining files
With this commit drm/amd/ has no longer any uses of the deprecated drmP.h header file. Signed-off-by: Sam Ravnborg Cc: Alex Deucher Cc: "Christian König" Cc: "David (ChunMing) Zhou" Cc: David Airlie Cc: Daniel Vetter --- drivers/gpu/drm/amd/amdgpu/atombios_crtc.c | 2 +- drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 2 +- drivers/gpu/drm/amd/amdgpu/atombios_encoders.c | 4 +++- drivers/gpu/drm/amd/amdgpu/atombios_i2c.c | 2 +- drivers/gpu/drm/amd/amdgpu/cik.c | 3 ++- drivers/gpu/drm/amd/amdgpu/cik_ih.c| 4 +++- drivers/gpu/drm/amd/amdgpu/cik_sdma.c | 4 +++- drivers/gpu/drm/amd/amdgpu/cz_ih.c | 4 +++- drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 5 - drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 5 - drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 7 ++- drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 5 - drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 4 +++- drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 4 +++- drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 3 ++- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 ++- drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 5 - drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 5 - drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 5 - drivers/gpu/drm/amd/amdgpu/iceland_ih.c| 4 +++- drivers/gpu/drm/amd/amdgpu/kv_dpm.c| 2 +- drivers/gpu/drm/amd/amdgpu/kv_smc.c| 2 +- drivers/gpu/drm/amd/amdgpu/psp_v3_1.c | 4 +++- drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c | 2 +- drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 5 +++-- drivers/gpu/drm/amd/amdgpu/si.c| 3 ++- drivers/gpu/drm/amd/amdgpu/si_dma.c| 2 +- drivers/gpu/drm/amd/amdgpu/si_dpm.c| 4 +++- drivers/gpu/drm/amd/amdgpu/si_ih.c | 4 +++- drivers/gpu/drm/amd/amdgpu/si_smc.c| 2 +- drivers/gpu/drm/amd/amdgpu/soc15.c | 3 ++- drivers/gpu/drm/amd/amdgpu/tonga_ih.c | 4 +++- drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c | 2 +- drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/vce_v2_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/vce_v4_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 4 +++- drivers/gpu/drm/amd/amdgpu/vi.c| 4 +++- 43 files changed, 99 insertions(+), 44 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_crtc.c b/drivers/gpu/drm/amd/amdgpu/atombios_crtc.c index 8a0818b23ea4..213e62a28ba0 100644 --- a/drivers/gpu/drm/amd/amdgpu/atombios_crtc.c +++ b/drivers/gpu/drm/amd/amdgpu/atombios_crtc.c @@ -23,7 +23,7 @@ * Authors: Dave Airlie * Alex Deucher */ -#include + #include #include #include diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c index f81068ba4cc6..6858cde9fc5d 100644 --- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c +++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c @@ -24,7 +24,7 @@ * Alex Deucher * Jerome Glisse */ -#include + #include #include "amdgpu.h" diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c index 60e2447e12c5..1e94a9b652f7 100644 --- a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c +++ b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c @@ -23,7 +23,9 @@ * Authors: Dave Airlie * Alex Deucher */ -#include + +#include + #include #include #include "amdgpu.h" diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_i2c.c b/drivers/gpu/drm/amd/amdgpu/atombios_i2c.c index f9b2ce9a98f3..980c363b1a0a 100644 --- a/drivers/gpu/drm/amd/amdgpu/atombios_i2c.c +++ b/drivers/gpu/drm/amd/amdgpu/atombios_i2c.c @@ -22,7 +22,7 @@ * Authors: Alex Deucher * */ -#include + #include #include "amdgpu.h" #include "atom.h" diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/amdgpu/cik.c index 07c1f239e9c3..a316ce8eec98 100644 --- a/drivers/gpu/drm/amd/amdgpu/cik.c +++ b/drivers/gpu/drm/amd/amdgpu/cik.c @@ -24,7 +24,8 @@ #include #include #include -#include +#include + #include "amdgpu.h" #include "amdgpu_atombios.h" #include "amdgpu_ih.h" diff --git a/drivers/gpu/drm/amd/amdgpu/cik_ih.c b/drivers/gpu/drm/amd/amdgpu/cik_ih.c index 721c757156e8..401c99f0b2d0 100644 --- a/drivers/gpu/drm/amd/amdgpu/cik_ih.c +++ b/drivers/gpu/drm/amd/amdgpu/cik_ih.c @@ -20,7 +20,9 @@ * OTHER DEALINGS IN THE SOFTWARE. * */ -#include + +#include + #include "amdgpu.h" #include "amdgpu_ih.h" #include "cikd.h" diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c index
[PATCH v1 02/10] drm/amd: drop dependencies on drm_os_linux.h
Fix so no files in drm/amd/ depends on the
deprecated drm_os_linux.h header file.
It was done manually:
- remove drm_os_linux.h from drmP.h
- fix all build errros
Signed-off-by: Sam Ravnborg
Cc: Alex Deucher
Cc: "Christian König"
Cc: "David (ChunMing) Zhou"
Cc: David Airlie
Cc: Daniel Vetter
---
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c| 4 +++-
drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 2 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 17 ++---
drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 8
drivers/gpu/drm/amd/amdgpu/cik_sdma.c | 2 +-
drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c | 2 +-
drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 2 +-
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 5 -
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 5 -
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c | 5 -
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 5 -
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 4 +++-
drivers/gpu/drm/amd/amdgpu/si_dma.c | 2 +-
drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c | 2 +-
drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c | 4 +++-
drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c | 4 ++--
drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c | 4 ++--
.../drm/amd/display/dc/core/dc_hw_sequencer.c | 2 ++
drivers/gpu/drm/amd/display/dc/core/dc_stream.c | 2 ++
drivers/gpu/drm/amd/display/dc/dc_helper.c | 3 +++
drivers/gpu/drm/amd/display/dc/dce/dce_aux.c| 2 ++
drivers/gpu/drm/amd/display/dc/dce/dce_dmcu.c | 2 ++
drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c | 3 +++
drivers/gpu/drm/amd/display/dc/dce/dce_i2c_sw.c | 3 +++
.../drm/amd/display/dc/dce/dce_link_encoder.c | 2 ++
.../drm/amd/display/dc/dce/dce_stream_encoder.c | 2 ++
.../amd/display/dc/dce110/dce110_compressor.c | 2 ++
.../amd/display/dc/dce110/dce110_hw_sequencer.c | 3 +++
.../display/dc/dce110/dce110_opp_regamma_v.c| 2 ++
.../amd/display/dc/dce110/dce110_transform_v.c | 2 ++
.../amd/display/dc/dce112/dce112_compressor.c | 2 ++
.../gpu/drm/amd/display/dc/dcn10/dcn10_hubbub.c | 2 ++
.../amd/display/dc/dcn10/dcn10_link_encoder.c | 2 ++
.../amd/display/dc/dcn10/dcn10_stream_encoder.c | 1 +
drivers/gpu/drm/amd/display/dc/gpio/hw_ddc.c| 2 ++
36 files changed, 90 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index a6e5184d436c..d2b51bc3f534 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -22,10 +22,12 @@
#define pr_fmt(fmt) "kfd2kgd: " fmt
+#include
#include
#include
#include
-#include
+#include
+
#include
#include "amdgpu_object.h"
#include "amdgpu_vm.h"
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 250d9212cc38..924d83e711ef 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -24,6 +24,8 @@
*
*/
+#include
+
#include "amdgpu.h"
/**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 0c52d1f9fe0f..e9ede34dd875 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -29,6 +29,16 @@
*Thomas Hellstrom
*Dave Airlie
*/
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
#include
#include
#include
@@ -36,13 +46,6 @@
#include
#include
#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
#include "amdgpu.h"
#include "amdgpu_object.h"
#include "amdgpu_trace.h"
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
index c021b114c8a4..fa03081c2f78 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
@@ -1090,7 +1090,7 @@ int amdgpu_vce_ring_test_ring(struct amdgpu_ring *ring)
for (i = 0; i < timeout; i++) {
if (amdgpu_ring_get_rptr(ring) != rptr)
break;
- DRM_UDELAY(1);
+ udelay(1);
}
if (i >= timeout)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index ecf6f96df2ad..7ed5d4e3884d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -446,7 +446,7 @@ int amdgpu_vcn_dec_ring_test_ring(struct amdgpu_ring *ring)
tmp = RREG32(SOC15_REG_OFFSET(UVD, 0, mmUVD_SCRATCH9));
if (tmp == 0xDEADBEEF)
break;
- DRM_UDELAY(1);
+ udelay(1);
}
if (i >= adev->usec_timeout)
@@ -608,7 +608,7 @@ int amdgpu_vcn_enc_ring_test_ring(struct amdgpu_ring
[PATCH v1 03/10] drm/amd: drop use of drmp.h in os_types.h
Drop use of the deprecated drmP.h from display/dc/os_types.h Fix all fallout after this change. Most of the fixes was adding a missing include of vmalloc.h. Signed-off-by: Sam Ravnborg Cc: Alex Deucher Cc: "Christian König" Cc: "David (ChunMing) Zhou" Cc: David Airlie Cc: Daniel Vetter --- drivers/gpu/drm/amd/display/dc/basics/vector.c| 2 ++ drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c| 2 ++ drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c | 2 ++ drivers/gpu/drm/amd/display/dc/core/dc.c | 2 ++ drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c | 2 ++ drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 3 +++ drivers/gpu/drm/amd/display/dc/core/dc_sink.c | 2 ++ drivers/gpu/drm/amd/display/dc/core/dc_stream.c | 1 + drivers/gpu/drm/amd/display/dc/core/dc_surface.c | 2 ++ drivers/gpu/drm/amd/display/dc/dce/dce_audio.c| 2 ++ drivers/gpu/drm/amd/display/dc/dce/dce_aux.c | 1 + drivers/gpu/drm/amd/display/dc/dce/dce_clk_mgr.c | 2 ++ drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c | 2 ++ drivers/gpu/drm/amd/display/dc/dce/dce_dmcu.c | 1 + drivers/gpu/drm/amd/display/dc/dce/dce_ipp.c | 2 ++ drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.c | 1 + drivers/gpu/drm/amd/display/dc/dce/dce_opp.c | 2 ++ drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c | 3 +++ drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c | 1 + drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c | 2 ++ drivers/gpu/drm/amd/display/dc/dce112/dce112_compressor.c | 1 + drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c | 2 ++ drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c | 2 ++ drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c | 2 ++ drivers/gpu/drm/amd/display/dc/dcn10/dcn10_clk_mgr.c | 2 ++ drivers/gpu/drm/amd/display/dc/dcn10/dcn10_ipp.c | 2 ++ drivers/gpu/drm/amd/display/dc/dcn10/dcn10_link_encoder.c | 1 + drivers/gpu/drm/amd/display/dc/dcn10/dcn10_opp.c | 2 ++ drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c | 2 ++ drivers/gpu/drm/amd/display/dc/gpio/gpio_base.c | 2 ++ drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c| 2 ++ drivers/gpu/drm/amd/display/dc/gpio/hw_ddc.c | 1 + drivers/gpu/drm/amd/display/dc/gpio/hw_factory.c | 2 ++ drivers/gpu/drm/amd/display/dc/gpio/hw_hpd.c | 2 ++ .../drm/amd/display/dc/irq/dce110/irq_service_dce110.c| 2 ++ .../drm/amd/display/dc/irq/dce120/irq_service_dce120.c| 2 ++ .../gpu/drm/amd/display/dc/irq/dce80/irq_service_dce80.c | 2 ++ .../gpu/drm/amd/display/dc/irq/dcn10/irq_service_dcn10.c | 2 ++ drivers/gpu/drm/amd/display/dc/irq/irq_service.c | 2 ++ drivers/gpu/drm/amd/display/dc/os_types.h | 8 +--- .../gpu/drm/amd/display/dc/virtual/virtual_link_encoder.c | 2 ++ .../drm/amd/display/dc/virtual/virtual_stream_encoder.c | 2 ++ drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 3 +++ drivers/gpu/drm/amd/display/modules/freesync/freesync.c | 2 ++ 44 files changed, 86 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/basics/vector.c b/drivers/gpu/drm/amd/display/dc/basics/vector.c index d28e9cf0e961..8f93d25f91ee 100644 --- a/drivers/gpu/drm/amd/display/dc/basics/vector.c +++ b/drivers/gpu/drm/amd/display/dc/basics/vector.c @@ -23,6 +23,8 @@ * */ +#include + #include "dm_services.h" #include "include/vector.h" diff --git a/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c b/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c index fd5266a58297..5e1b849684a6 100644 --- a/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c +++ b/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c @@ -23,6 +23,8 @@ * */ +#include + #include "dm_services.h" #include "ObjectID.h" diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c b/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c index f3aa7b53d2aa..7108d51a9c5b 100644 --- a/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c +++ b/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c @@ -23,6 +23,8 @@ * */ +#include + #include "dm_services.h" #include "dce_calcs.h" #include "dc.h" diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c index 18c775a950cc..03dec40de361 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c @@ -22,6 +22,8 @@ * Authors: AMD */ +#include + #include "dm_services.h" #include "dc.h" 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 f02092a0dc76..eecc631ca4f8 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 @@ -23,6 +23,8 @@ * */ +#include +
[PATCH v1 0/10] drm/amd: drop use of drmP.h
This patcset drop all uses of drm_os_linux.h and drmP.h in drm/amd/. The patchset depends on the earlier series removing drmP.h from drm/radeon. https://lists.freedesktop.org/archives/dri-devel/2019-June/220969.html The only dependency os the patch to drm_debugfs.h: https://lists.freedesktop.org/archives/dri-devel/2019-June/220971.html The removal was done in a number of steps, mainly to easy potential reviews and to allow some parts to be applied if not everything are OK. The patches are made on top of drm-misc-next. There is a single patch touching drm_print.h - this was needed to prevent adding include of to a lot of files, because it is required by one of the macros in drm_print.h. As this patch only adds an include file, it should be straightforward to apply. All patches are build tested with various configs and various architectures. In a few cases the include of header files was re-arranged, but in general the changes are kept to a minimum. When adding new include files the different blocks of include failes are seperated by empty lines. This account for some of the added lines. Sam Sam Ravnborg (10): drm: fix build errors with drm_print.h drm/amd: drop dependencies on drm_os_linux.h drm/amd: drop use of drmp.h in os_types.h drm/amd: drop use of drmP.h in amdgpu.h drm/amd: drop use of drmP.h in atom.h drm/amd: drop use of drmP.h from all header files drm/amd: drop use of drmP.h in powerplay/ drm/amd: drop use of drmP.h in display/ drm/amd: drop use of drmP.h in amdgpu/amdgpu* drm/amd: drop use of drmP.h in remaining files drivers/gpu/drm/amd/amdgpu/amdgpu.h | 4 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_afmt.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 6 -- drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 7 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 5 - drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 14 +++--- drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c| 4 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 5 - drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 2 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_i2c.c | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 4 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_ioc32.c| 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 5 - drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 5 - drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 5 - drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 2 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c| 3 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_sched.h| 5 - drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_test.c | 2 +-
[PATCH v1 07/10] drm/amd: drop use of drmP.h in powerplay/
Delete the only include of drmP.h in powerplay/. Signed-off-by: Sam Ravnborg Cc: Alex Deucher Cc: "Christian König" Cc: "David (ChunMing) Zhou" Cc: David Airlie Cc: Daniel Vetter --- drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c index eec329ab6037..a4c9d9267f8e 100644 --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c @@ -20,9 +20,9 @@ * OTHER DEALINGS IN THE SOFTWARE. */ -#include "pp_debug.h" #include -#include + +#include "pp_debug.h" #include "amdgpu.h" #include "amdgpu_smu.h" #include "soc15_common.h" -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1 04/10] drm/amd: drop use of drmP.h in amdgpu.h
Delete the unused drmP.h from amdgpu.h. Fix fallout in various files. Signed-off-by: Sam Ravnborg Cc: Alex Deucher Cc: "Christian König" Cc: "David (ChunMing) Zhou" Cc: David Airlie Cc: Daniel Vetter --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 4 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 2 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c| 3 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 4 drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c| 2 ++ drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c| 4 drivers/gpu/drm/amd/amdgpu/psp_v10_0.c | 3 +++ drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 2 ++ drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c| 1 + drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c| 4 +++- drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 1 + drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 1 + drivers/gpu/drm/amd/powerplay/hwmgr/vega10_processpptables.c | 1 + drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c | 1 + drivers/gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c| 1 + drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c | 2 ++ drivers/gpu/drm/amd/powerplay/smumgr/smu10_smumgr.c | 2 ++ drivers/gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c | 1 + drivers/gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c | 2 ++ 19 files changed, 38 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 14398f55f602..fbec83bfb4ad 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -44,9 +44,9 @@ #include #include -#include -#include #include +#include +#include #include #include diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index 22bd21efe6b1..4fea7f835506 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -24,6 +24,8 @@ #include #include #include +#include + #include "amdgpu.h" #include "amdgpu_ras.h" #include "amdgpu_atomfirmware.h" diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c index 639297250c21..c799691dfa84 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c @@ -23,8 +23,11 @@ */ #include +#include #include + #include + #include "amdgpu.h" #include "amdgpu_vm.h" diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c index 7d484fad3909..01f88269a7f9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c @@ -21,6 +21,10 @@ * */ +#include + +#include + #include "amdgpu.h" bool amdgpu_virt_mmio_blocked(struct amdgpu_device *adev) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c index 4f761ebb655e..91f10995249b 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c @@ -21,6 +21,8 @@ * */ #include +#include + #include "amdgpu.h" #include "amdgpu_ih.h" #include "amdgpu_gfx.h" diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c index 3b7370d914a5..51bbf773b4f5 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c @@ -20,8 +20,12 @@ * OTHER DEALINGS IN THE SOFTWARE. * */ + #include +#include + #include + #include "amdgpu.h" #include "gmc_v9_0.h" #include "amdgpu_atomfirmware.h" diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c index 77c2bc344dfc..ce1ea31feee0 100644 --- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c +++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c @@ -24,6 +24,9 @@ */ #include +#include +#include + #include "amdgpu.h" #include "amdgpu_psp.h" #include "amdgpu_ucode.h" diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c index b91df7bd1d98..b1e7aca72578 100644 --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c @@ -21,6 +21,8 @@ */ #include +#include + #include "amdgpu.h" #include "amdgpu_psp.h" #include "amdgpu_ucode.h" diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c index a10e3a50d9ef..bc67e6502733 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c @@ -24,6 +24,7 @@ */ #include +#include #include "amdgpu.h" #include "amdgpu_dm.h" diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c index 1d5fc5ad3bee..e611b5376d8c
[PATCH v1 01/10] drm: fix build errors with drm_print.h
drm_print.h requires to fix build when macros are used. Pull in the header file in drm_print.h so users do not have to do it. Signed-off-by: Sam Ravnborg Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Daniel Vetter --- include/drm/drm_print.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index 3a4247319e63..a5d6f2f3e430 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -32,6 +32,8 @@ #include #include +#include + /** * DOC: print * -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] mm/hmm: pass mmu_notifier_range to sync_cpu_device_pagetables
On Fri, Jun 07, 2019 at 05:14:52PM -0700, Ralph Campbell wrote:
> HMM defines its own struct hmm_update which is passed to the
> sync_cpu_device_pagetables() callback function. This is
> sufficient when the only action is to invalidate. However,
> a device may want to know the reason for the invalidation and
> be able to see the new permissions on a range, update device access
> rights or range statistics. Since sync_cpu_device_pagetables()
> can be called from try_to_unmap(), the mmap_sem may not be held
> and find_vma() is not safe to be called.
> Pass the struct mmu_notifier_range to sync_cpu_device_pagetables()
> to allow the full invalidation information to be used.
>
> Signed-off-by: Ralph Campbell
I don't disagree with Christoph or Jason but since I've been trying to sort out
where hmm does and does not fit any chance to remove a custom structure is a
good simplification IMO. So...
Reviewed-by: Ira Weiny
> ---
>
> I'm sending this out now since we are updating many of the HMM APIs
> and I think it will be useful.
>
>
> drivers/gpu/drm/nouveau/nouveau_svm.c | 4 ++--
> include/linux/hmm.h | 27 ++-
> mm/hmm.c | 13 -
> 3 files changed, 8 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c
> b/drivers/gpu/drm/nouveau/nouveau_svm.c
> index 8c92374afcf2..c34b98fafe2f 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> @@ -252,13 +252,13 @@ nouveau_svmm_invalidate(struct nouveau_svmm *svmm, u64
> start, u64 limit)
>
> static int
> nouveau_svmm_sync_cpu_device_pagetables(struct hmm_mirror *mirror,
> - const struct hmm_update *update)
> + const struct mmu_notifier_range *update)
> {
> struct nouveau_svmm *svmm = container_of(mirror, typeof(*svmm), mirror);
> unsigned long start = update->start;
> unsigned long limit = update->end;
>
> - if (!update->blockable)
> + if (!mmu_notifier_range_blockable(update))
> return -EAGAIN;
>
> SVMM_DBG(svmm, "invalidate %016lx-%016lx", start, limit);
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index 0fa8ea34ccef..07a2d38fde34 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -377,29 +377,6 @@ static inline uint64_t hmm_pfn_from_pfn(const struct
> hmm_range *range,
>
> struct hmm_mirror;
>
> -/*
> - * enum hmm_update_event - type of update
> - * @HMM_UPDATE_INVALIDATE: invalidate range (no indication as to why)
> - */
> -enum hmm_update_event {
> - HMM_UPDATE_INVALIDATE,
> -};
> -
> -/*
> - * struct hmm_update - HMM update information for callback
> - *
> - * @start: virtual start address of the range to update
> - * @end: virtual end address of the range to update
> - * @event: event triggering the update (what is happening)
> - * @blockable: can the callback block/sleep ?
> - */
> -struct hmm_update {
> - unsigned long start;
> - unsigned long end;
> - enum hmm_update_event event;
> - bool blockable;
> -};
> -
> /*
> * struct hmm_mirror_ops - HMM mirror device operations callback
> *
> @@ -420,7 +397,7 @@ struct hmm_mirror_ops {
> /* sync_cpu_device_pagetables() - synchronize page tables
>*
>* @mirror: pointer to struct hmm_mirror
> - * @update: update information (see struct hmm_update)
> + * @update: update information (see struct mmu_notifier_range)
>* Return: -EAGAIN if update.blockable false and callback need to
>* block, 0 otherwise.
>*
> @@ -434,7 +411,7 @@ struct hmm_mirror_ops {
>* synchronous call.
>*/
> int (*sync_cpu_device_pagetables)(struct hmm_mirror *mirror,
> - const struct hmm_update *update);
> + const struct mmu_notifier_range *update);
> };
>
> /*
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 9aad3550f2bb..b49a43712554 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -164,7 +164,6 @@ static int hmm_invalidate_range_start(struct mmu_notifier
> *mn,
> {
> struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
> struct hmm_mirror *mirror;
> - struct hmm_update update;
> struct hmm_range *range;
> unsigned long flags;
> int ret = 0;
> @@ -173,15 +172,10 @@ static int hmm_invalidate_range_start(struct
> mmu_notifier *mn,
> if (!kref_get_unless_zero(>kref))
> return 0;
>
> - update.start = nrange->start;
> - update.end = nrange->end;
> - update.event = HMM_UPDATE_INVALIDATE;
> - update.blockable = mmu_notifier_range_blockable(nrange);
> -
> spin_lock_irqsave(>ranges_lock, flags);
> hmm->notifiers++;
> list_for_each_entry(range, >ranges, list) {
> - if (update.end < range->start || update.start >= range->end)
> + if
[Bug 109955] amdgpu [RX Vega 64] system freeze while gaming
https://bugs.freedesktop.org/show_bug.cgi?id=109955 --- Comment #29 from Sam --- I have been trying myself for the moment to get some info with just debug parameters: amdgpu.dc=1 amdgpu.vm_fault_stop=2 amdgpu.vm_debug=1 amdgpu.gpu_recovery=0 Incidentally I couldn't get any freeze to happen after running two troublesome games for about two hours each (left idle but on load, Pillars of Eternity and Surviving Mars) but this could mean anything as they happen completely randomly. Perhaps someone who can reproduce the issue instantly can test the parameters more reliably? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/34] treewide: simplify getting the adapter of an I2C client
On 2019-06-08 12:55, Wolfram Sang wrote: > While preparing a refactoring series, I noticed that some drivers use a > complicated way of determining the adapter of a client. The easy way is > to use the intended pointer: client->adapter > > These drivers do: > to_i2c_adapter(client->dev.parent); > > The I2C core populates the parent pointer as: > client->dev.parent = >adapter->dev; > > Now take into consideration that > to_i2c_adapter(>dev); > > is a complicated way of saying 'adapter', then we can even formally > prove that the complicated expression can be simplified by using > client->adapter. > > The conversion was done using a coccinelle script with some manual > indentation fixes applied on top. > > To avoid a brown paper bag mistake, I double checked this on a Renesas > Salvator-XS board (R-Car M3N) and verified both expression result in the > same pointer. Other than that, the series is only build tested. Similar things go on in: drivers/hwmon/lm90.c drivers/leds/leds-is31fl319x.c drivers/of/unittest.c Those have this pattern: struct device *dev = >dev; struct i2c_adapter *adapter = to_i2c_adapter(dev->parent); And drivers/rtc/rtc-fm3130.c has a couple of these: tmp = i2c_transfer(to_i2c_adapter(fm3130->client->dev.parent), ...); where fm3130->client is of type "struct i2c_client *" Cheers, Peter
[Bug 110866] Revert 8059add0478e29cb641936011a8fcc9ce9fd80be for stable 5.1.x
https://bugs.freedesktop.org/show_bug.cgi?id=110866 Bug ID: 110866 Summary: Revert 8059add0478e29cb641936011a8fcc9ce9fd80be for stable 5.1.x Product: DRI Version: unspecified Hardware: Other URL: https://bugzilla.kernel.org/show_bug.cgi?id=203679 OS: All Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: nrn...@gmail.com Kernel 5.0 marked as EOL and 5.1 is broken for RADV VR on Steam. Copy of description from kernel bugzilla: Original commit 8059add0478e29cb641936011a8fcc9ce9fd80be (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/drm_ioctl.c?id=8059add0478e29cb641936011a8fcc9ce9fd80be) was added in 5.1 merge cycle and reverted by dbb92471674a48892f5e50779425e03388073ab9 (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=dbb92471674a48892f5e50779425e03388073ab9) in 5.2. So radv is still broken in 5.1. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110865] Rx480 consumes 20w more power in idle than under Windows
https://bugs.freedesktop.org/show_bug.cgi?id=110865 Bug ID: 110865 Summary: Rx480 consumes 20w more power in idle than under Windows Product: Mesa Version: 19.1 Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/radeonsi Assignee: dri-devel@lists.freedesktop.org Reporter: mw...@adiumentum.com QA Contact: dri-devel@lists.freedesktop.org Created attachment 144485 --> https://bugs.freedesktop.org/attachment.cgi?id=144485=edit logfiles as requested in the amd bugreport guide First I am not sure where to file that bug, so please be gentle with me, if I selected the wrong component. I noticed for a while higher temperatures of my Videocard when my pc was just idling with gnome. Then I dug deeper and found out that my "zero fan" videocard does not stop the fan when I run Linux. So I ran this line here: watch -n 0.5 cat /sys/kernel/debug/dri/0/amdgpu_pm_info and it showed me that the MCLK does not clock down to 300MHz as it does with Windows 10. GFX Clocks and Power: 2000 MHz (MCLK) 300 MHz (SCLK) 300 MHz (PSTATE_SCLK) 300 MHz (PSTATE_MCLK) 1000 mV (VDDGFX) 24.75 W (average GPU) GPU Temperature: 45 C GPU Load: 0 % I have a multimonitor setup with two 1920x1200 pixel screens. When I use Windows 10, the MCLK does not go beyond 300MHz when the desktop is idling. (measured with hwmonitor) When I power-off one screen under linux the (average GPU) goes down to 8-10W and the MCLK drops to 300MHz, so the card can clock down, but is somehow prohibited by the driver or configuration? I followed this bug report guide from amd: https://www.amd.com/en/support/kb/faq/amdgpu-installation#faq-Reporting-Bugs and attached several logfiles. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 08/11] mm/hmm: Remove racy protection against double-unregistration
On Thu, Jun 06, 2019 at 08:29:10PM -0700, John Hubbard wrote:
> On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
> > From: Jason Gunthorpe
> >
> > No other register/unregister kernel API attempts to provide this kind of
> > protection as it is inherently racy, so just drop it.
> >
> > Callers should provide their own protection, it appears nouveau already
> > does, but just in case drop a debugging POISON.
> >
> > Signed-off-by: Jason Gunthorpe
> > Reviewed-by: Jérôme Glisse
> > mm/hmm.c | 9 ++---
> > 1 file changed, 2 insertions(+), 7 deletions(-)
> >
> > diff --git a/mm/hmm.c b/mm/hmm.c
> > index c702cd72651b53..6802de7080d172 100644
> > +++ b/mm/hmm.c
> > @@ -284,18 +284,13 @@ EXPORT_SYMBOL(hmm_mirror_register);
> > */
> > void hmm_mirror_unregister(struct hmm_mirror *mirror)
> > {
> > - struct hmm *hmm = READ_ONCE(mirror->hmm);
> > -
> > - if (hmm == NULL)
> > - return;
> > + struct hmm *hmm = mirror->hmm;
> >
> > down_write(>mirrors_sem);
> > list_del_init(>list);
> > - /* To protect us against double unregister ... */
> > - mirror->hmm = NULL;
> > up_write(>mirrors_sem);
> > -
> > hmm_put(hmm);
> > + memset(>hmm, POISON_INUSE, sizeof(mirror->hmm));
>
> I hadn't thought of POISON_* for these types of cases, it's a
> good technique to remember.
>
> I noticed that this is now done outside of the lock, but that
> follows directly from your commit description, so that all looks
> correct.
Yes, the thing about POISON is that if you ever read it then you have
found a use after free bug - thus we should never need to write it
under a lock (just after a serializing lock)
Normally I wouldn't bother as kfree does poison as well, but since we
can't easily audit the patches yet to be submitted this seems safer
and will reliably cause those patches to explode with an oops in
testing.
Thanks,
Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 05/11] mm/hmm: Remove duplicate condition test before wait_event_timeout
On Thu, Jun 06, 2019 at 08:06:52PM -0700, John Hubbard wrote:
> On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
> > From: Jason Gunthorpe
> >
> > The wait_event_timeout macro already tests the condition as its first
> > action, so there is no reason to open code another version of this, all
> > that does is skip the might_sleep() debugging in common cases, which is
> > not helpful.
> >
> > Further, based on prior patches, we can no simplify the required condition
>
> "now simplify"
>
> > test:
> > - If range is valid memory then so is range->hmm
> > - If hmm_release() has run then range->valid is set to false
> >at the same time as dead, so no reason to check both.
> > - A valid hmm has a valid hmm->mm.
> >
> > Also, add the READ_ONCE for range->valid as there is no lock held here.
> >
> > Signed-off-by: Jason Gunthorpe
> > Reviewed-by: Jérôme Glisse
> > include/linux/hmm.h | 12 ++--
> > 1 file changed, 2 insertions(+), 10 deletions(-)
> >
> > diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> > index 4ee3acabe5ed22..2ab35b40992b24 100644
> > +++ b/include/linux/hmm.h
> > @@ -218,17 +218,9 @@ static inline unsigned long hmm_range_page_size(const
> > struct hmm_range *range)
> > static inline bool hmm_range_wait_until_valid(struct hmm_range *range,
> > unsigned long timeout)
> > {
> > - /* Check if mm is dead ? */
> > - if (range->hmm == NULL || range->hmm->dead || range->hmm->mm == NULL) {
> > - range->valid = false;
> > - return false;
> > - }
> > - if (range->valid)
> > - return true;
> > - wait_event_timeout(range->hmm->wq, range->valid || range->hmm->dead,
> > + wait_event_timeout(range->hmm->wq, range->valid,
> >msecs_to_jiffies(timeout));
> > - /* Return current valid status just in case we get lucky */
> > - return range->valid;
> > + return READ_ONCE(range->valid);
>
> Just to ensure that I actually understand the model: I'm assuming that the
> READ_ONCE is there solely to ensure that range->valid is read *after* the
> wait_event_timeout() returns. Is that correct?
No, wait_event_timout already has internal barriers that make sure
things don't leak across it.
The READ_ONCE is required any time a thread is reading a value that
another thread can be concurrently changing - ie in this case there is
no lock protecting range->valid so the write side could be running.
Without the READ_ONCE the compiler is allowed to read the value twice
and assume it gets the same result, which may not be true with a
parallel writer, and thus may compromise the control flow in some
unknown way.
It is also good documentation for the locking scheme in use as it
marks shared data that is not being locked.
However, now that dead is gone we can just write the above more simply
as:
static inline bool hmm_range_wait_until_valid(struct hmm_range *range,
unsigned long timeout)
{
return wait_event_timeout(range->hmm->wq, range->valid,
msecs_to_jiffies(timeout)) != 0;
}
Which relies on the internal barriers of wait_event_timeout, I'll fix
it up..
Thanks,
Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 05/11] mm/hmm: Remove duplicate condition test before wait_event_timeout
On 6/7/19 1:44 PM, Jason Gunthorpe wrote:
On Fri, Jun 07, 2019 at 01:21:12PM -0700, Ralph Campbell wrote:
What I want to get to is a pattern like this:
pagefault():
hmm_range_register();
again:
/* On the slow path, if we appear to be live locked then we get
the write side of mmap_sem which will break the live lock,
otherwise this gets the read lock */
if (hmm_range_start_and_lock())
goto err;
lockdep_assert_held(range->mm->mmap_sem);
// Optional: Avoid useless expensive work
if (hmm_range_needs_retry())
goto again;
hmm_range_(touch vmas)
take_lock(driver->update);
if (hmm_range_end() {
release_lock(driver->update);
goto again;
}
// Finish driver updates
release_lock(driver->update);
// Releases mmap_sem
hmm_range_unregister_and_unlock();
What do you think?
Is it clear?
Jason
Are you talking about acquiring mmap_sem in hmm_range_start_and_lock()?
Usually, the fault code has to lock mmap_sem for read in order to
call find_vma() so it can set range.vma.
If HMM drops mmap_sem - which I don't think it should, just return an
error to tell the caller to drop mmap_sem and retry - the find_vma()
will need to be repeated as well.
Overall I don't think it makes a lot of sense to sleep for retry in
hmm_range_start_and_lock() while holding mmap_sem. It would be better
to drop that lock, sleep, then re-acquire it as part of the hmm logic.
The find_vma should be done inside the critical section created by
hmm_range_start_and_lock(), not before it. If we are retrying then we
already slept and the additional CPU cost to repeat the find_vma is
immaterial, IMHO?
Do you see a reason why the find_vma() ever needs to be before the
'again' in my above example? range.vma does not need to be set for
range_register.
Yes, for the GPU case, there can be many faults in an event queue
and the goal is to try to handle more than one page at a time.
The vma is needed to limit the amount of coalescing and checking
for pages that could be speculatively migrated or mapped.
I'm also not sure about acquiring the mmap_sem for write as way to
mitigate thrashing. It seems to me that if a device and a CPU are
both faulting on the same page,
One of the reasons to prefer this approach is that it means we don't
need to keep track of which ranges we are faulting, and if there is a
lot of *unrelated* fault activity (unlikely?) we can resolve it using
mmap sem instead of this elaborate ranges scheme and related
locking.
This would reduce the overall work in the page fault and
invalidate_start/end paths for the common uncontended cases.
some sort of backoff delay is needed to let one side or the other
make some progress.
What the write side of the mmap_sem would do is force the CPU and
device to cleanly take turns. Once the device pages are registered
under the write side the CPU will have to wait in invalidate_start for
the driver to complete a shootdown, then the whole thing starts all
over again.
It is certainly imaginable something could have a 'min life' timer for
a device mapping and hold mm invalidate_start, and device pagefault
for that min time to promote better sharing.
But, if we don't use the mmap_sem then we can livelock and the device
will see an unrecoverable error from the timeout which means we have
risk that under load the system will simply obscurely fail. This seems
unacceptable to me..
Particularly since for the ODP use case the issue is not trashing
migration as a GPU might have, but simple system stability under swap
load. We do not want the ODP pagefault to permanently fail due to
timeout if the VMA is still valid..
Jason
OK, I understand.
If you come up with a set of changes, I can try testing them.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 05/11] mm/hmm: Remove duplicate condition test before wait_event_timeout
On 6/7/19 12:13 PM, Jason Gunthorpe wrote:
On Fri, Jun 07, 2019 at 12:01:45PM -0700, Ralph Campbell wrote:
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
From: Jason Gunthorpe
The wait_event_timeout macro already tests the condition as its first
action, so there is no reason to open code another version of this, all
that does is skip the might_sleep() debugging in common cases, which is
not helpful.
Further, based on prior patches, we can no simplify the required condition
test:
- If range is valid memory then so is range->hmm
- If hmm_release() has run then range->valid is set to false
at the same time as dead, so no reason to check both.
- A valid hmm has a valid hmm->mm.
Also, add the READ_ONCE for range->valid as there is no lock held here.
Signed-off-by: Jason Gunthorpe
Reviewed-by: Jérôme Glisse
include/linux/hmm.h | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 4ee3acabe5ed22..2ab35b40992b24 100644
+++ b/include/linux/hmm.h
@@ -218,17 +218,9 @@ static inline unsigned long hmm_range_page_size(const
struct hmm_range *range)
static inline bool hmm_range_wait_until_valid(struct hmm_range *range,
unsigned long timeout)
{
- /* Check if mm is dead ? */
- if (range->hmm == NULL || range->hmm->dead || range->hmm->mm == NULL) {
- range->valid = false;
- return false;
- }
- if (range->valid)
- return true;
- wait_event_timeout(range->hmm->wq, range->valid || range->hmm->dead,
+ wait_event_timeout(range->hmm->wq, range->valid,
msecs_to_jiffies(timeout));
- /* Return current valid status just in case we get lucky */
- return range->valid;
+ return READ_ONCE(range->valid);
}
/*
Since we are simplifying things, perhaps we should consider merging
hmm_range_wait_until_valid() info hmm_range_register() and
removing hmm_range_wait_until_valid() since the pattern
is to always call the two together.
? the hmm.rst shows the hmm_range_wait_until_valid being called in the
(ret == -EAGAIN) path. It is confusing because it should really just
have the again label moved up above hmm_range_wait_until_valid() as
even if we get the driver lock it could still be a long wait for the
colliding invalidation to clear.
What I want to get to is a pattern like this:
pagefault():
hmm_range_register();
again:
/* On the slow path, if we appear to be live locked then we get
the write side of mmap_sem which will break the live lock,
otherwise this gets the read lock */
if (hmm_range_start_and_lock())
goto err;
lockdep_assert_held(range->mm->mmap_sem);
// Optional: Avoid useless expensive work
if (hmm_range_needs_retry())
goto again;
hmm_range_(touch vmas)
take_lock(driver->update);
if (hmm_range_end() {
release_lock(driver->update);
goto again;
}
// Finish driver updates
release_lock(driver->update);
// Releases mmap_sem
hmm_range_unregister_and_unlock();
What do you think?
Is it clear?
Jason
Are you talking about acquiring mmap_sem in hmm_range_start_and_lock()?
Usually, the fault code has to lock mmap_sem for read in order to
call find_vma() so it can set range.vma.
If HMM drops mmap_sem - which I don't think it should, just return an
error to tell the caller to drop mmap_sem and retry - the find_vma()
will need to be repeated as well.
I'm also not sure about acquiring the mmap_sem for write as way to
mitigate thrashing. It seems to me that if a device and a CPU are
both faulting on the same page, some sort of backoff delay is needed
to let one side or the other make some progress.
Thrashing mitigation and how migrate_vma() plays in this is a
deep topic for thought.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 01/11] mm/hmm: fix use after free with struct hmm in the mmu notifiers
On Thu, Jun 06, 2019 at 07:29:08PM -0700, John Hubbard wrote:
> On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
> > From: Jason Gunthorpe
> ...
> > diff --git a/mm/hmm.c b/mm/hmm.c
> > index 8e7403f081f44a..547002f56a163d 100644
> > +++ b/mm/hmm.c
> ...
> > @@ -125,7 +130,7 @@ static void hmm_free(struct kref *kref)
> > mm->hmm = NULL;
> > spin_unlock(>page_table_lock);
> >
> > - kfree(hmm);
> > + mmu_notifier_call_srcu(>rcu, hmm_free_rcu);
>
>
> It occurred to me to wonder if it is best to use the MMU notifier's
> instance of srcu, instead of creating a separate instance for HMM.
It *has* to be the MMU notifier SRCU because we are synchornizing
against the read side of that SRU inside the mmu notifier code, ie:
int __mmu_notifier_invalidate_range_start(struct mmu_notifier_range *range)
id = srcu_read_lock();
hlist_for_each_entry_rcu(mn, >mm->mmu_notifier_mm->list, hlist) {
if (mn->ops->invalidate_range_start) {
^
Here 'mn' is really hmm (hmm = container_of(mn, struct hmm,
mmu_notifier)), so we must protect the memory against free for the mmu
notifier core.
Thus we have no choice but to use its SRCU.
CH also pointed out a more elegant solution, which is to get the write
side of the mmap_sem during hmm_mirror_unregister - no notifier
callback can be running in this case. Then we delete the kref, srcu
and so forth.
This is much clearer/saner/better, but.. requries the callers of
hmm_mirror_unregister to be safe to get the mmap_sem write side.
I think this is true, so maybe this patch should be switched, what do
you think?
> > @@ -153,10 +158,14 @@ void hmm_mm_destroy(struct mm_struct *mm)
> >
> > static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
> > {
> > - struct hmm *hmm = mm_get_hmm(mm);
> > + struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
> > struct hmm_mirror *mirror;
> > struct hmm_range *range;
> >
> > + /* hmm is in progress to free */
>
> Well, sometimes, yes. :)
It think it is in all cases actually.. The only way we see a 0 kref
and still reach this code path is if another thread has alreay setup
the hmm_free in the call_srcu..
> Maybe this wording is clearer (if we need any comment at all):
I always find this hard.. This is a very standard pattern when working
with RCU - however in my experience few people actually know the RCU
patterns, and missing the _unless_zero is a common bug I find when
looking at code.
This is mm/ so I can drop it, what do you think?
Thanks,
Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 02/34] gpu: drm: bridge: sii9234: simplify getting the adapter of a client
We have a dedicated pointer for that, so use it. Much easier to read and
less computation involved.
Signed-off-by: Wolfram Sang
---
Please apply to your subsystem tree.
drivers/gpu/drm/bridge/sii9234.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii9234.c b/drivers/gpu/drm/bridge/sii9234.c
index b36bbafb0e43..25d4ad8c7ad6 100644
--- a/drivers/gpu/drm/bridge/sii9234.c
+++ b/drivers/gpu/drm/bridge/sii9234.c
@@ -815,7 +815,7 @@ static irqreturn_t sii9234_irq_thread(int irq, void *data)
static int sii9234_init_resources(struct sii9234 *ctx,
struct i2c_client *client)
{
- struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
+ struct i2c_adapter *adapter = client->adapter;
int ret;
if (!ctx->dev->of_node) {
@@ -897,7 +897,7 @@ static const struct drm_bridge_funcs sii9234_bridge_funcs =
{
static int sii9234_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
- struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
+ struct i2c_adapter *adapter = client->adapter;
struct sii9234 *ctx;
struct device *dev = >dev;
int ret;
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 05/11] mm/hmm: Remove duplicate condition test before wait_event_timeout
On Fri, Jun 07, 2019 at 01:21:12PM -0700, Ralph Campbell wrote:
> > What I want to get to is a pattern like this:
> >
> > pagefault():
> >
> > hmm_range_register();
> > again:
> > /* On the slow path, if we appear to be live locked then we get
> >the write side of mmap_sem which will break the live lock,
> >otherwise this gets the read lock */
> > if (hmm_range_start_and_lock())
> > goto err;
> >
> > lockdep_assert_held(range->mm->mmap_sem);
> >
> > // Optional: Avoid useless expensive work
> > if (hmm_range_needs_retry())
> >goto again;
> > hmm_range_(touch vmas)
> >
> > take_lock(driver->update);
> > if (hmm_range_end() {
> > release_lock(driver->update);
> > goto again;
> > }
> > // Finish driver updates
> > release_lock(driver->update);
> >
> > // Releases mmap_sem
> > hmm_range_unregister_and_unlock();
> >
> > What do you think?
> >
> > Is it clear?
> >
> > Jason
> >
>
> Are you talking about acquiring mmap_sem in hmm_range_start_and_lock()?
> Usually, the fault code has to lock mmap_sem for read in order to
> call find_vma() so it can set range.vma.
> If HMM drops mmap_sem - which I don't think it should, just return an
> error to tell the caller to drop mmap_sem and retry - the find_vma()
> will need to be repeated as well.
Overall I don't think it makes a lot of sense to sleep for retry in
hmm_range_start_and_lock() while holding mmap_sem. It would be better
to drop that lock, sleep, then re-acquire it as part of the hmm logic.
The find_vma should be done inside the critical section created by
hmm_range_start_and_lock(), not before it. If we are retrying then we
already slept and the additional CPU cost to repeat the find_vma is
immaterial, IMHO?
Do you see a reason why the find_vma() ever needs to be before the
'again' in my above example? range.vma does not need to be set for
range_register.
> I'm also not sure about acquiring the mmap_sem for write as way to
> mitigate thrashing. It seems to me that if a device and a CPU are
> both faulting on the same page,
One of the reasons to prefer this approach is that it means we don't
need to keep track of which ranges we are faulting, and if there is a
lot of *unrelated* fault activity (unlikely?) we can resolve it using
mmap sem instead of this elaborate ranges scheme and related
locking.
This would reduce the overall work in the page fault and
invalidate_start/end paths for the common uncontended cases.
> some sort of backoff delay is needed to let one side or the other
> make some progress.
What the write side of the mmap_sem would do is force the CPU and
device to cleanly take turns. Once the device pages are registered
under the write side the CPU will have to wait in invalidate_start for
the driver to complete a shootdown, then the whole thing starts all
over again.
It is certainly imaginable something could have a 'min life' timer for
a device mapping and hold mm invalidate_start, and device pagefault
for that min time to promote better sharing.
But, if we don't use the mmap_sem then we can livelock and the device
will see an unrecoverable error from the timeout which means we have
risk that under load the system will simply obscurely fail. This seems
unacceptable to me..
Particularly since for the ODP use case the issue is not trashing
migration as a GPU might have, but simple system stability under swap
load. We do not want the ODP pagefault to permanently fail due to
timeout if the VMA is still valid..
Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 03/11] mm/hmm: Hold a mmgrab from hmm to mm
On Thu, Jun 06, 2019 at 07:44:58PM -0700, John Hubbard wrote:
> On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
> > From: Jason Gunthorpe
> >
> > So long a a struct hmm pointer exists, so should the struct mm it is
> > linked too. Hold the mmgrab() as soon as a hmm is created, and mmdrop() it
> > once the hmm refcount goes to zero.
> >
> > Since mmdrop() (ie a 0 kref on struct mm) is now impossible with a !NULL
> > mm->hmm delete the hmm_hmm_destroy().
> >
> > Signed-off-by: Jason Gunthorpe
> > Reviewed-by: Jérôme Glisse
> > v2:
> > - Fix error unwind paths in hmm_get_or_create (Jerome/Jason)
> > include/linux/hmm.h | 3 ---
> > kernel/fork.c | 1 -
> > mm/hmm.c| 22 --
> > 3 files changed, 4 insertions(+), 22 deletions(-)
> >
> > diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> > index 2d519797cb134a..4ee3acabe5ed22 100644
> > +++ b/include/linux/hmm.h
> > @@ -586,14 +586,11 @@ static inline int hmm_vma_fault(struct hmm_mirror
> > *mirror,
> > }
> >
> > /* Below are for HMM internal use only! Not to be used by device driver! */
> > -void hmm_mm_destroy(struct mm_struct *mm);
> > -
> > static inline void hmm_mm_init(struct mm_struct *mm)
> > {
> > mm->hmm = NULL;
> > }
> > #else /* IS_ENABLED(CONFIG_HMM_MIRROR) */
> > -static inline void hmm_mm_destroy(struct mm_struct *mm) {}
> > static inline void hmm_mm_init(struct mm_struct *mm) {}
> > #endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
> >
> > diff --git a/kernel/fork.c b/kernel/fork.c
> > index b2b87d450b80b5..588c768ae72451 100644
> > +++ b/kernel/fork.c
> > @@ -673,7 +673,6 @@ void __mmdrop(struct mm_struct *mm)
> > WARN_ON_ONCE(mm == current->active_mm);
> > mm_free_pgd(mm);
> > destroy_context(mm);
> > - hmm_mm_destroy(mm);
>
>
> This is particularly welcome, not to have an "HMM is special" case
> in such a core part of process/mm code.
I would very much like to propose something like 'per-net' for struct
mm, as rdma also need to add some data to each mm to make it's use of
mmu notifiers work (for basically this same reason as HMM)
Thanks,
Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 09/11] mm/hmm: Poison hmm_range during unregister
On Thu, Jun 06, 2019 at 08:37:42PM -0700, John Hubbard wrote: > On 6/6/19 11:44 AM, Jason Gunthorpe wrote: > > From: Jason Gunthorpe > > > > Trying to misuse a range outside its lifetime is a kernel bug. Use WARN_ON > > and poison bytes to detect this condition. > > > > Signed-off-by: Jason Gunthorpe > > Reviewed-by: Jérôme Glisse > > v2 > > - Keep range start/end valid after unregistration (Jerome) > > mm/hmm.c | 7 +-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/mm/hmm.c b/mm/hmm.c > > index 6802de7080d172..c2fecb3ecb11e1 100644 > > +++ b/mm/hmm.c > > @@ -937,7 +937,7 @@ void hmm_range_unregister(struct hmm_range *range) > > struct hmm *hmm = range->hmm; > > > > /* Sanity check this really should not happen. */ > > That comment can also be deleted, as it has the same meaning as > the WARN_ON() that you just added. > > > - if (hmm == NULL || range->end <= range->start) > > + if (WARN_ON(range->end <= range->start)) > > return; > > > > mutex_lock(>lock); > > @@ -948,7 +948,10 @@ void hmm_range_unregister(struct hmm_range *range) > > range->valid = false; > > mmput(hmm->mm); > > hmm_put(hmm); > > - range->hmm = NULL; > > + > > + /* The range is now invalid, leave it poisoned. */ > > To be precise, we are poisoning the range's back pointer to it's > owning hmm instance. Maybe this is clearer: > > /* >* The range is now invalid, so poison it's hmm pointer. >* Leave other range-> fields in place, for the caller's use. >*/ > > ...or something like that? > > > + range->valid = false; > > + memset(>hmm, POISON_INUSE, sizeof(range->hmm)); > > } > > EXPORT_SYMBOL(hmm_range_unregister); > > > > > > The above are very minor documentation points, so: > > Reviewed-by: John Hubbard done thanks Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 12/11] mm/hmm: Fix error flows in hmm_invalidate_range_start
On Fri, Jun 07, 2019 at 04:52:58PM -0700, Ralph Campbell wrote:
> > @@ -141,6 +142,23 @@ static void hmm_release(struct mmu_notifier *mn,
> > struct mm_struct *mm)
> > hmm_put(hmm);
> > }
> > +static void notifiers_decrement(struct hmm *hmm)
> > +{
> > + lockdep_assert_held(>ranges_lock);
> > +
> > + hmm->notifiers--;
> > + if (!hmm->notifiers) {
> > + struct hmm_range *range;
> > +
> > + list_for_each_entry(range, >ranges, list) {
> > + if (range->valid)
> > + continue;
> > + range->valid = true;
> > + }
>
> This just effectively sets all ranges to valid.
> I'm not sure that is best.
This is a trade off, it would be much more expensive to have a precise
'valid = true' - instead this algorithm is precise about 'valid =
false' and lazy about 'valid = true' which is much less costly to
calculate.
> Shouldn't hmm_range_register() start with range.valid = true and
> then hmm_invalidate_range_start() set affected ranges to false?
It kind of does, expect when it doesn't, right? :)
> Then this becomes just wake_up_all() if --notifiers == 0 and
> hmm_range_wait_until_valid() should wait for notifiers == 0.
Almost.. but it is more tricky than that.
This scheme is a collision-retry algorithm. The pagefault side runs to
completion if no parallel invalidate start/end happens.
If a parallel invalidation happens then the pagefault retries.
Seeing notifiers == 0 means there is absolutely no current parallel
invalidation.
Seeing range->valid == true (under the device lock)
means this range doesn't intersect with a parallel invalidate.
So.. hmm_range_wait_until_valid() checks the per-range valid because
it doesn't want to sleep if *this range* is not involved in a parallel
invalidation - but once it becomes involved, then yes, valid == true
implies notifiers == 0.
It is easier/safer to use unlocked variable reads if there is only one
variable, thus the weird construction.
It is unclear to me if this micro optimization is really
worthwhile. It is very expensive to manage all this tracking, and no
other mmu notifier implementation really does something like
this. Eliminating the per-range tracking and using the notifier count
as a global lock would be much simpler...
> Otherwise, range.valid doesn't really mean it's valid.
Right, it doesn't really mean 'valid'
It is tracking possible colliding invalidates such that valid == true
(under the device lock) means that there was no colliding invalidate.
I still think this implementation doesn't quite work, as I described
here:
https://lore.kernel.org/linux-mm/20190527195829.gb18...@mellanox.com/
But the idea is basically sound and matches what other mmu notifier
users do, just using a seqcount like scheme, not a boolean.
Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm: add fallback override/firmware EDID modes workaround
On Fri, 2019-06-07 at 17:10 +0200, Daniel Vetter wrote: > As discussed on irc, we need tested-by here from the reporters since > there's way too many losing and frustrangingly few winning moves here. Tested-by: Paul Wise I've tested these two patches on top of Linux v5.2-rc3 and the EDID override works correctly on an Intel Ironlake GPU with a monitor that lost its EDID a while ago. I'll test that it also works with an nVidia GPU & noveau drivers later today once that system is available. https://patchwork.freedesktop.org/series/61764/ -- bye, pabs https://bonedaddy.net/pabs3/ signature.asc Description: This is a digitally signed message part ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 10/11] mm/hmm: Do not use list*_rcu() for hmm->ranges
On 6/6/19 11:44 AM, Jason Gunthorpe wrote: From: Jason Gunthorpe This list is always read and written while holding hmm->lock so there is no need for the confusing _rcu annotations. Signed-off-by: Jason Gunthorpe Reviewed-by: Jérôme Glisse Reviewed-by: Ralph Campbell --- mm/hmm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mm/hmm.c b/mm/hmm.c index c2fecb3ecb11e1..709d138dd49027 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -911,7 +911,7 @@ int hmm_range_register(struct hmm_range *range, mutex_lock(>lock); range->hmm = hmm; - list_add_rcu(>list, >ranges); + list_add(>list, >ranges); /* * If there are any concurrent notifiers we have to wait for them for @@ -941,7 +941,7 @@ void hmm_range_unregister(struct hmm_range *range) return; mutex_lock(>lock); - list_del_rcu(>list); + list_del(>list); mutex_unlock(>lock); /* Drop reference taken by hmm_range_register() */ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 02/11] mm/hmm: Use hmm_mirror not mm as an argument for hmm_range_register
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
From: Jason Gunthorpe
Ralph observes that hmm_range_register() can only be called by a driver
while a mirror is registered. Make this clear in the API by passing in the
mirror structure as a parameter.
This also simplifies understanding the lifetime model for struct hmm, as
the hmm pointer must be valid as part of a registered mirror so all we
need in hmm_register_range() is a simple kref_get.
Suggested-by: Ralph Campbell
Signed-off-by: Jason Gunthorpe
You might CC Ben for the nouveau part.
CC: Ben Skeggs
Reviewed-by: Ralph Campbell
---
v2
- Include the oneline patch to nouveau_svm.c
---
drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +-
include/linux/hmm.h | 7 ---
mm/hmm.c | 15 ++-
3 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c
b/drivers/gpu/drm/nouveau/nouveau_svm.c
index 93ed43c413f0bb..8c92374afcf227 100644
--- a/drivers/gpu/drm/nouveau/nouveau_svm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
@@ -649,7 +649,7 @@ nouveau_svm_fault(struct nvif_notify *notify)
range.values = nouveau_svm_pfn_values;
range.pfn_shift = NVIF_VMM_PFNMAP_V0_ADDR_SHIFT;
again:
- ret = hmm_vma_fault(, true);
+ ret = hmm_vma_fault(>mirror, , true);
if (ret == 0) {
mutex_lock(>mutex);
if (!hmm_vma_range_done()) {
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 688c5ca7068795..2d519797cb134a 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -505,7 +505,7 @@ static inline bool hmm_mirror_mm_is_alive(struct hmm_mirror
*mirror)
* Please see Documentation/vm/hmm.rst for how to use the range API.
*/
int hmm_range_register(struct hmm_range *range,
- struct mm_struct *mm,
+ struct hmm_mirror *mirror,
unsigned long start,
unsigned long end,
unsigned page_shift);
@@ -541,7 +541,8 @@ static inline bool hmm_vma_range_done(struct hmm_range
*range)
}
/* This is a temporary helper to avoid merge conflict between trees. */
-static inline int hmm_vma_fault(struct hmm_range *range, bool block)
+static inline int hmm_vma_fault(struct hmm_mirror *mirror,
+ struct hmm_range *range, bool block)
{
long ret;
@@ -554,7 +555,7 @@ static inline int hmm_vma_fault(struct hmm_range *range, bool block)
range->default_flags = 0;
range->pfn_flags_mask = -1UL;
- ret = hmm_range_register(range, range->vma->vm_mm,
+ ret = hmm_range_register(range, mirror,
range->start, range->end,
PAGE_SHIFT);
if (ret)
diff --git a/mm/hmm.c b/mm/hmm.c
index 547002f56a163d..8796447299023c 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -925,13 +925,13 @@ static void hmm_pfns_clear(struct hmm_range *range,
* Track updates to the CPU page table see include/linux/hmm.h
*/
int hmm_range_register(struct hmm_range *range,
- struct mm_struct *mm,
+ struct hmm_mirror *mirror,
unsigned long start,
unsigned long end,
unsigned page_shift)
{
unsigned long mask = ((1UL << page_shift) - 1UL);
- struct hmm *hmm;
+ struct hmm *hmm = mirror->hmm;
range->valid = false;
range->hmm = NULL;
@@ -945,15 +945,12 @@ int hmm_range_register(struct hmm_range *range,
range->start = start;
range->end = end;
- hmm = hmm_get_or_create(mm);
- if (!hmm)
- return -EFAULT;
-
/* Check if hmm_mm_destroy() was call. */
- if (hmm->mm == NULL || hmm->dead) {
- hmm_put(hmm);
+ if (hmm->mm == NULL || hmm->dead)
return -EFAULT;
- }
+
+ range->hmm = hmm;
+ kref_get(>kref);
/* Initialize range to track CPU page table updates. */
mutex_lock(>lock);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 10/11] mm/hmm: Do not use list*_rcu() for hmm->ranges
On Fri, Jun 7, 2019 at 12:15 AM Jason Gunthorpe wrote: > > From: Jason Gunthorpe > > This list is always read and written while holding hmm->lock so there is > no need for the confusing _rcu annotations. > > Signed-off-by: Jason Gunthorpe > Reviewed-by: Jérôme Glisse Acked-by: Souptick Joarder > --- > mm/hmm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/mm/hmm.c b/mm/hmm.c > index c2fecb3ecb11e1..709d138dd49027 100644 > --- a/mm/hmm.c > +++ b/mm/hmm.c > @@ -911,7 +911,7 @@ int hmm_range_register(struct hmm_range *range, > mutex_lock(>lock); > > range->hmm = hmm; > - list_add_rcu(>list, >ranges); > + list_add(>list, >ranges); > > /* > * If there are any concurrent notifiers we have to wait for them for > @@ -941,7 +941,7 @@ void hmm_range_unregister(struct hmm_range *range) > return; > > mutex_lock(>lock); > - list_del_rcu(>list); > + list_del(>list); > mutex_unlock(>lock); > > /* Drop reference taken by hmm_range_register() */ > -- > 2.21.0 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 03/11] mm/hmm: Hold a mmgrab from hmm to mm
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
From: Jason Gunthorpe
So long a a struct hmm pointer exists, so should the struct mm it is
s/a a/as a/
linked too. Hold the mmgrab() as soon as a hmm is created, and mmdrop() it
once the hmm refcount goes to zero.
Since mmdrop() (ie a 0 kref on struct mm) is now impossible with a !NULL
mm->hmm delete the hmm_hmm_destroy().
Signed-off-by: Jason Gunthorpe
Reviewed-by: Jérôme Glisse
Reviewed-by: Ralph Campbell
---
v2:
- Fix error unwind paths in hmm_get_or_create (Jerome/Jason)
---
include/linux/hmm.h | 3 ---
kernel/fork.c | 1 -
mm/hmm.c| 22 --
3 files changed, 4 insertions(+), 22 deletions(-)
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 2d519797cb134a..4ee3acabe5ed22 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -586,14 +586,11 @@ static inline int hmm_vma_fault(struct hmm_mirror *mirror,
}
/* Below are for HMM internal use only! Not to be used by device driver! */
-void hmm_mm_destroy(struct mm_struct *mm);
-
static inline void hmm_mm_init(struct mm_struct *mm)
{
mm->hmm = NULL;
}
#else /* IS_ENABLED(CONFIG_HMM_MIRROR) */
-static inline void hmm_mm_destroy(struct mm_struct *mm) {}
static inline void hmm_mm_init(struct mm_struct *mm) {}
#endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
diff --git a/kernel/fork.c b/kernel/fork.c
index b2b87d450b80b5..588c768ae72451 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -673,7 +673,6 @@ void __mmdrop(struct mm_struct *mm)
WARN_ON_ONCE(mm == current->active_mm);
mm_free_pgd(mm);
destroy_context(mm);
- hmm_mm_destroy(mm);
mmu_notifier_mm_destroy(mm);
check_mm(mm);
put_user_ns(mm->user_ns);
diff --git a/mm/hmm.c b/mm/hmm.c
index 8796447299023c..cc7c26fda3300e 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -82,6 +83,7 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
hmm->notifiers = 0;
hmm->dead = false;
hmm->mm = mm;
+ mmgrab(hmm->mm);
spin_lock(>page_table_lock);
if (!mm->hmm)
@@ -109,6 +111,7 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
mm->hmm = NULL;
spin_unlock(>page_table_lock);
error:
+ mmdrop(hmm->mm);
kfree(hmm);
return NULL;
}
@@ -130,6 +133,7 @@ static void hmm_free(struct kref *kref)
mm->hmm = NULL;
spin_unlock(>page_table_lock);
+ mmdrop(hmm->mm);
mmu_notifier_call_srcu(>rcu, hmm_free_rcu);
}
@@ -138,24 +142,6 @@ static inline void hmm_put(struct hmm *hmm)
kref_put(>kref, hmm_free);
}
-void hmm_mm_destroy(struct mm_struct *mm)
-{
- struct hmm *hmm;
-
- spin_lock(>page_table_lock);
- hmm = mm_get_hmm(mm);
- mm->hmm = NULL;
- if (hmm) {
- hmm->mm = NULL;
- hmm->dead = true;
- spin_unlock(>page_table_lock);
- hmm_put(hmm);
- return;
- }
-
- spin_unlock(>page_table_lock);
-}
-
static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
{
struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 01/11] mm/hmm: fix use after free with struct hmm in the mmu notifiers
On 6/7/19 5:34 AM, Jason Gunthorpe wrote:
> On Thu, Jun 06, 2019 at 07:29:08PM -0700, John Hubbard wrote:
>> On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
>>> From: Jason Gunthorpe
>> ...
>>> diff --git a/mm/hmm.c b/mm/hmm.c
>>> index 8e7403f081f44a..547002f56a163d 100644
>>> +++ b/mm/hmm.c
>> ...
>>> @@ -125,7 +130,7 @@ static void hmm_free(struct kref *kref)
>>> mm->hmm = NULL;
>>> spin_unlock(>page_table_lock);
>>>
>>> - kfree(hmm);
>>> + mmu_notifier_call_srcu(>rcu, hmm_free_rcu);
>>
>>
>> It occurred to me to wonder if it is best to use the MMU notifier's
>> instance of srcu, instead of creating a separate instance for HMM.
>
> It *has* to be the MMU notifier SRCU because we are synchornizing
> against the read side of that SRU inside the mmu notifier code, ie:
>
> int __mmu_notifier_invalidate_range_start(struct mmu_notifier_range *range)
> id = srcu_read_lock();
> hlist_for_each_entry_rcu(mn, >mm->mmu_notifier_mm->list,
> hlist) {
> if (mn->ops->invalidate_range_start) {
>^
>
> Here 'mn' is really hmm (hmm = container_of(mn, struct hmm,
> mmu_notifier)), so we must protect the memory against free for the mmu
> notifier core.
>
> Thus we have no choice but to use its SRCU.
Ah right. It's embarassingly obvious when you say it out loud. :)
Thanks for explaining.
>
> CH also pointed out a more elegant solution, which is to get the write
> side of the mmap_sem during hmm_mirror_unregister - no notifier
> callback can be running in this case. Then we delete the kref, srcu
> and so forth.
>
> This is much clearer/saner/better, but.. requries the callers of
> hmm_mirror_unregister to be safe to get the mmap_sem write side.
>
> I think this is true, so maybe this patch should be switched, what do
> you think?
OK, your follow-up notes that we'll leave it as is, got it.
thanks,
--
John Hubbard
NVIDIA
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 07/11] mm/hmm: Use lockdep instead of comments
On Fri, Jun 7, 2019 at 12:15 AM Jason Gunthorpe wrote:
>
> From: Jason Gunthorpe
>
> So we can check locking at runtime.
Little more descriptive change log would be helpful.
Acked-by: Souptick Joarder
>
> Signed-off-by: Jason Gunthorpe
> Reviewed-by: Jérôme Glisse
> ---
> v2
> - Fix missing & in lockdeps (Jason)
> ---
> mm/hmm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/hmm.c b/mm/hmm.c
> index f67ba32983d9f1..c702cd72651b53 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -254,11 +254,11 @@ static const struct mmu_notifier_ops
> hmm_mmu_notifier_ops = {
> *
> * To start mirroring a process address space, the device driver must
> register
> * an HMM mirror struct.
> - *
> - * THE mm->mmap_sem MUST BE HELD IN WRITE MODE !
> */
> int hmm_mirror_register(struct hmm_mirror *mirror, struct mm_struct *mm)
> {
> + lockdep_assert_held_exclusive(>mmap_sem);
> +
> /* Sanity check */
> if (!mm || !mirror || !mirror->ops)
> return -EINVAL;
> --
> 2.21.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 08/11] mm/hmm: Remove racy protection against double-unregistration
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
From: Jason Gunthorpe
No other register/unregister kernel API attempts to provide this kind of
protection as it is inherently racy, so just drop it.
Callers should provide their own protection, it appears nouveau already
does, but just in case drop a debugging POISON.
Signed-off-by: Jason Gunthorpe
Reviewed-by: Jérôme Glisse
Reviewed-by: Ralph Campbell
---
mm/hmm.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/mm/hmm.c b/mm/hmm.c
index c702cd72651b53..6802de7080d172 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -284,18 +284,13 @@ EXPORT_SYMBOL(hmm_mirror_register);
*/
void hmm_mirror_unregister(struct hmm_mirror *mirror)
{
- struct hmm *hmm = READ_ONCE(mirror->hmm);
-
- if (hmm == NULL)
- return;
+ struct hmm *hmm = mirror->hmm;
down_write(>mirrors_sem);
list_del_init(>list);
- /* To protect us against double unregister ... */
- mirror->hmm = NULL;
up_write(>mirrors_sem);
-
hmm_put(hmm);
+ memset(>hmm, POISON_INUSE, sizeof(mirror->hmm));
}
EXPORT_SYMBOL(hmm_mirror_unregister);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] mm/hmm: pass mmu_notifier_range to sync_cpu_device_pagetables
On Sat, Jun 08, 2019 at 02:10:08AM -0700, Christoph Hellwig wrote: > On Fri, Jun 07, 2019 at 05:14:52PM -0700, Ralph Campbell wrote: > > HMM defines its own struct hmm_update which is passed to the > > sync_cpu_device_pagetables() callback function. This is > > sufficient when the only action is to invalidate. However, > > a device may want to know the reason for the invalidation and > > be able to see the new permissions on a range, update device access > > rights or range statistics. Since sync_cpu_device_pagetables() > > can be called from try_to_unmap(), the mmap_sem may not be held > > and find_vma() is not safe to be called. > > Pass the struct mmu_notifier_range to sync_cpu_device_pagetables() > > to allow the full invalidation information to be used. > > > > Signed-off-by: Ralph Campbell > > > > I'm sending this out now since we are updating many of the HMM APIs > > and I think it will be useful. > > This is the right thing to do. But the really right thing is to just > kill the hmm_mirror API entirely and move to mmu_notifiers. At least > for noveau this already is way simpler, although right now it defeats > Jasons patch to avoid allocating the struct hmm in the fault path. > But as said before that can be avoided by just killing struct hmm, > which for many reasons is the right thing to do anyway. > > I've got a series here, which is a bit broken (epecially the last > patch can't work as-is), but should explain where I'm trying to head: > > http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/hmm-mirror-simplification At least the current hmm approach does rely on the collision retry locking scheme in struct hmm/struct hmm_range for the pagefault side to work right. So, before we can apply patch one in this series we need to fix hmm_vma_fault() and all its varients. Otherwise the driver will be broken. I'm hoping to first define what this locking should be (see other emails to Ralph) then, ideally, see if we can extend mmu notifiers to get it directly withouth hmm stuff. Then we apply your patch one and the hmm ops wrapper dies. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 09/11] mm/hmm: Poison hmm_range during unregister
On Fri, Jun 07, 2019 at 01:46:30PM -0700, Ralph Campbell wrote: > > On 6/6/19 11:44 AM, Jason Gunthorpe wrote: > > From: Jason Gunthorpe > > > > Trying to misuse a range outside its lifetime is a kernel bug. Use WARN_ON > > and poison bytes to detect this condition. > > > > Signed-off-by: Jason Gunthorpe > > Reviewed-by: Jérôme Glisse > > Reviewed-by: Ralph Campbell > > > v2 > > - Keep range start/end valid after unregistration (Jerome) > > mm/hmm.c | 7 +-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/mm/hmm.c b/mm/hmm.c > > index 6802de7080d172..c2fecb3ecb11e1 100644 > > +++ b/mm/hmm.c > > @@ -937,7 +937,7 @@ void hmm_range_unregister(struct hmm_range *range) > > struct hmm *hmm = range->hmm; > > /* Sanity check this really should not happen. */ > > - if (hmm == NULL || range->end <= range->start) > > + if (WARN_ON(range->end <= range->start)) > > return; > > WARN_ON() is definitely better than silent return but I wonder how > useful it is since the caller shouldn't be modifying the hmm_range > once it is registered. Other fields could be changed too... I deleted the warn on (see the other thread), but I'm confused by your "shouldn't be modified" statement. The only thing that needs to be set and remain unchanged for register is the virtual start/end address. Everything else should be done once it is clear to proceed based on the collision-retry locking scheme this uses. Basically the range register only setups a 'detector' for colliding invalidations. The other stuff in the struct is just random temporary storage for the API. AFAICS at least.. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 11/11] mm/hmm: Remove confusing comment and logic from hmm_release
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
From: Jason Gunthorpe
hmm_release() is called exactly once per hmm. ops->release() cannot
accidentally trigger any action that would recurse back onto
hmm->mirrors_sem.
This fixes a use after-free race of the form:
CPU0 CPU1
hmm_release()
up_write(>mirrors_sem);
hmm_mirror_unregister(mirror)
down_write(>mirrors_sem);
up_write(>mirrors_sem);
kfree(mirror)
mirror->ops->release(mirror)
The only user we have today for ops->release is an empty function, so this
is unambiguously safe.
As a consequence of plugging this race drivers are not allowed to
register/unregister mirrors from within a release op.
Signed-off-by: Jason Gunthorpe
I agree with the analysis above but I'm not sure that release() will
always be an empty function. It might be more efficient to write back
all data migrated to a device "in one pass" instead of relying
on unmap_vmas() calling hmm_start_range_invalidate() per VMA.
I think the bigger issue is potential deadlocks while calling
sync_cpu_device_pagetables() and tasks calling hmm_mirror_unregister():
Say you have three threads:
- Thread A is in try_to_unmap(), either without holding mmap_sem or with
mmap_sem held for read.
- Thread B has some unrelated driver calling hmm_mirror_unregister().
This doesn't require mmap_sem.
- Thread C is about to call migrate_vma().
Thread AThread B Thread C
try_to_unmaphmm_mirror_unregistermigrate_vma
-- --- --
hmm_invalidate_range_start
down_read(mirrors_sem)
down_write(mirrors_sem)
// Blocked on A
device_lock
device_lock
// Blocked on C
migrate_vma()
hmm_invalidate_range_s
down_read(mirrors_sem)
// Blocked on B
// Deadlock
Perhaps we should consider using SRCU for walking the mirror->list?
---
mm/hmm.c | 28 +---
1 file changed, 9 insertions(+), 19 deletions(-)
diff --git a/mm/hmm.c b/mm/hmm.c
index 709d138dd49027..3a45dd3d778248 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -136,26 +136,16 @@ static void hmm_release(struct mmu_notifier *mn, struct
mm_struct *mm)
WARN_ON(!list_empty(>ranges));
mutex_unlock(>lock);
- down_write(>mirrors_sem);
- mirror = list_first_entry_or_null(>mirrors, struct hmm_mirror,
- list);
- while (mirror) {
- list_del_init(>list);
- if (mirror->ops->release) {
- /*
-* Drop mirrors_sem so the release callback can wait
-* on any pending work that might itself trigger a
-* mmu_notifier callback and thus would deadlock with
-* us.
-*/
- up_write(>mirrors_sem);
+ down_read(>mirrors_sem);
+ list_for_each_entry(mirror, >mirrors, list) {
+ /*
+* Note: The driver is not allowed to trigger
+* hmm_mirror_unregister() from this thread.
+*/
+ if (mirror->ops->release)
mirror->ops->release(mirror);
- down_write(>mirrors_sem);
- }
- mirror = list_first_entry_or_null(>mirrors,
- struct hmm_mirror, list);
}
- up_write(>mirrors_sem);
+ up_read(>mirrors_sem);
hmm_put(hmm);
}
@@ -287,7 +277,7 @@ void hmm_mirror_unregister(struct hmm_mirror *mirror)
struct hmm *hmm = mirror->hmm;
down_write(>mirrors_sem);
- list_del_init(>list);
+ list_del(>list);
up_write(>mirrors_sem);
hmm_put(hmm);
memset(>hmm, POISON_INUSE, sizeof(mirror->hmm));
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 11/11] mm/hmm: Remove confusing comment and logic from hmm_release
On Thu, Jun 06, 2019 at 08:47:28PM -0700, John Hubbard wrote:
> On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
> > From: Jason Gunthorpe
> >
> > hmm_release() is called exactly once per hmm. ops->release() cannot
> > accidentally trigger any action that would recurse back onto
> > hmm->mirrors_sem.
> >
> > This fixes a use after-free race of the form:
> >
> >CPU0 CPU1
> >hmm_release()
> > up_write(>mirrors_sem);
> > hmm_mirror_unregister(mirror)
> > down_write(>mirrors_sem);
> > up_write(>mirrors_sem);
> > kfree(mirror)
> > mirror->ops->release(mirror)
> >
> > The only user we have today for ops->release is an empty function, so this
> > is unambiguously safe.
> >
> > As a consequence of plugging this race drivers are not allowed to
> > register/unregister mirrors from within a release op.
> >
> > Signed-off-by: Jason Gunthorpe
> > mm/hmm.c | 28 +---
> > 1 file changed, 9 insertions(+), 19 deletions(-)
> >
> > diff --git a/mm/hmm.c b/mm/hmm.c
> > index 709d138dd49027..3a45dd3d778248 100644
> > +++ b/mm/hmm.c
> > @@ -136,26 +136,16 @@ static void hmm_release(struct mmu_notifier *mn,
> > struct mm_struct *mm)
> > WARN_ON(!list_empty(>ranges));
> > mutex_unlock(>lock);
> >
> > - down_write(>mirrors_sem);
> > - mirror = list_first_entry_or_null(>mirrors, struct hmm_mirror,
> > - list);
> > - while (mirror) {
> > - list_del_init(>list);
> > - if (mirror->ops->release) {
> > - /*
> > -* Drop mirrors_sem so the release callback can wait
> > -* on any pending work that might itself trigger a
> > -* mmu_notifier callback and thus would deadlock with
> > -* us.
> > -*/
> > - up_write(>mirrors_sem);
> > + down_read(>mirrors_sem);
>
> This is cleaner and simpler, but I suspect it is leading to the deadlock
> that Ralph Campbell is seeing in his driver testing. (And in general, holding
> a lock during a driver callback usually leads to deadlocks.)
I think Ralph has never seen this patch (it is new), so it must be one
of the earlier patches..
> Ralph, is this the one? It's the only place in this patchset where I can
> see a lock around a callback to driver code, that wasn't there before. So
> I'm pretty sure it is the one...
Can you share the lockdep report please?
Thanks,
Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 01/11] mm/hmm: fix use after free with struct hmm in the mmu notifiers
On Fri, Jun 07, 2019 at 09:34:32AM -0300, Jason Gunthorpe wrote: > CH also pointed out a more elegant solution, which is to get the write > side of the mmap_sem during hmm_mirror_unregister - no notifier > callback can be running in this case. Then we delete the kref, srcu > and so forth. Oops, it turns out this is only the case for invalidate_start/end, not release, so this doesn't help with the SRCU unless we also change exit_mmap to call release with the mmap sem held. So I think we have to stick with this for now. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC] mm/hmm: pass mmu_notifier_range to sync_cpu_device_pagetables
HMM defines its own struct hmm_update which is passed to the
sync_cpu_device_pagetables() callback function. This is
sufficient when the only action is to invalidate. However,
a device may want to know the reason for the invalidation and
be able to see the new permissions on a range, update device access
rights or range statistics. Since sync_cpu_device_pagetables()
can be called from try_to_unmap(), the mmap_sem may not be held
and find_vma() is not safe to be called.
Pass the struct mmu_notifier_range to sync_cpu_device_pagetables()
to allow the full invalidation information to be used.
Signed-off-by: Ralph Campbell
---
I'm sending this out now since we are updating many of the HMM APIs
and I think it will be useful.
drivers/gpu/drm/nouveau/nouveau_svm.c | 4 ++--
include/linux/hmm.h | 27 ++-
mm/hmm.c | 13 -
3 files changed, 8 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c
b/drivers/gpu/drm/nouveau/nouveau_svm.c
index 8c92374afcf2..c34b98fafe2f 100644
--- a/drivers/gpu/drm/nouveau/nouveau_svm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
@@ -252,13 +252,13 @@ nouveau_svmm_invalidate(struct nouveau_svmm *svmm, u64
start, u64 limit)
static int
nouveau_svmm_sync_cpu_device_pagetables(struct hmm_mirror *mirror,
- const struct hmm_update *update)
+ const struct mmu_notifier_range *update)
{
struct nouveau_svmm *svmm = container_of(mirror, typeof(*svmm), mirror);
unsigned long start = update->start;
unsigned long limit = update->end;
- if (!update->blockable)
+ if (!mmu_notifier_range_blockable(update))
return -EAGAIN;
SVMM_DBG(svmm, "invalidate %016lx-%016lx", start, limit);
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 0fa8ea34ccef..07a2d38fde34 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -377,29 +377,6 @@ static inline uint64_t hmm_pfn_from_pfn(const struct
hmm_range *range,
struct hmm_mirror;
-/*
- * enum hmm_update_event - type of update
- * @HMM_UPDATE_INVALIDATE: invalidate range (no indication as to why)
- */
-enum hmm_update_event {
- HMM_UPDATE_INVALIDATE,
-};
-
-/*
- * struct hmm_update - HMM update information for callback
- *
- * @start: virtual start address of the range to update
- * @end: virtual end address of the range to update
- * @event: event triggering the update (what is happening)
- * @blockable: can the callback block/sleep ?
- */
-struct hmm_update {
- unsigned long start;
- unsigned long end;
- enum hmm_update_event event;
- bool blockable;
-};
-
/*
* struct hmm_mirror_ops - HMM mirror device operations callback
*
@@ -420,7 +397,7 @@ struct hmm_mirror_ops {
/* sync_cpu_device_pagetables() - synchronize page tables
*
* @mirror: pointer to struct hmm_mirror
-* @update: update information (see struct hmm_update)
+* @update: update information (see struct mmu_notifier_range)
* Return: -EAGAIN if update.blockable false and callback need to
* block, 0 otherwise.
*
@@ -434,7 +411,7 @@ struct hmm_mirror_ops {
* synchronous call.
*/
int (*sync_cpu_device_pagetables)(struct hmm_mirror *mirror,
- const struct hmm_update *update);
+ const struct mmu_notifier_range *update);
};
/*
diff --git a/mm/hmm.c b/mm/hmm.c
index 9aad3550f2bb..b49a43712554 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -164,7 +164,6 @@ static int hmm_invalidate_range_start(struct mmu_notifier
*mn,
{
struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
struct hmm_mirror *mirror;
- struct hmm_update update;
struct hmm_range *range;
unsigned long flags;
int ret = 0;
@@ -173,15 +172,10 @@ static int hmm_invalidate_range_start(struct mmu_notifier
*mn,
if (!kref_get_unless_zero(>kref))
return 0;
- update.start = nrange->start;
- update.end = nrange->end;
- update.event = HMM_UPDATE_INVALIDATE;
- update.blockable = mmu_notifier_range_blockable(nrange);
-
spin_lock_irqsave(>ranges_lock, flags);
hmm->notifiers++;
list_for_each_entry(range, >ranges, list) {
- if (update.end < range->start || update.start >= range->end)
+ if (nrange->end < range->start || nrange->start >= range->end)
continue;
range->valid = false;
@@ -198,9 +192,10 @@ static int hmm_invalidate_range_start(struct mmu_notifier
*mn,
list_for_each_entry(mirror, >mirrors, list) {
int rc;
- rc = mirror->ops->sync_cpu_device_pagetables(mirror, );
+ rc = mirror->ops->sync_cpu_device_pagetables(mirror, nrange);
Re: [PATCH v3 hmm 05/11] mm/hmm: Remove duplicate condition test before wait_event_timeout
On 6/7/19 6:31 AM, Jason Gunthorpe wrote:
> The wait_event_timeout macro already tests the condition as its first
> action, so there is no reason to open code another version of this, all
> that does is skip the might_sleep() debugging in common cases, which is
> not helpful.
>
> Further, based on prior patches, we can now simplify the required condition
> test:
> - If range is valid memory then so is range->hmm
> - If hmm_release() has run then range->valid is set to false
>at the same time as dead, so no reason to check both.
> - A valid hmm has a valid hmm->mm.
>
> Allowing the return value of wait_event_timeout() (along with its internal
> barriers) to compute the result of the function.
>
> Signed-off-by: Jason Gunthorpe
> ---
Reviewed-by: John Hubbard
thanks,
--
John Hubbard
NVIDIA
> v3
> - Simplify the wait_event_timeout to not check valid
> ---
> include/linux/hmm.h | 13 ++---
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index 1d97b6d62c5bcf..26e7c477490c4e 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -209,17 +209,8 @@ static inline unsigned long hmm_range_page_size(const
> struct hmm_range *range)
> static inline bool hmm_range_wait_until_valid(struct hmm_range *range,
> unsigned long timeout)
> {
> - /* Check if mm is dead ? */
> - if (range->hmm == NULL || range->hmm->dead || range->hmm->mm == NULL) {
> - range->valid = false;
> - return false;
> - }
> - if (range->valid)
> - return true;
> - wait_event_timeout(range->hmm->wq, range->valid || range->hmm->dead,
> -msecs_to_jiffies(timeout));
> - /* Return current valid status just in case we get lucky */
> - return range->valid;
> + return wait_event_timeout(range->hmm->wq, range->valid,
> + msecs_to_jiffies(timeout)) != 0;
> }
>
> /*
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm: add fallback override/firmware EDID modes workaround
On Sat, 2019-06-08 at 13:10 +0800, Paul Wise wrote: > I'll test that it also works with an nVidia GPU & noveau drivers > later today once that system is available. Same results as with the Intel GPU: Correct screen resolution but missing EDID override data. -- bye, pabs https://bonedaddy.net/pabs3/ signature.asc Description: This is a digitally signed message part ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 04/11] mm/hmm: Simplify hmm_get_or_create and make it reliable
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
From: Jason Gunthorpe
As coded this function can false-fail in various racy situations. Make it
reliable by running only under the write side of the mmap_sem and avoiding
the false-failing compare/exchange pattern.
Also make the locking very easy to understand by only ever reading or
writing mm->hmm while holding the write side of the mmap_sem.
Signed-off-by: Jason Gunthorpe
Reviewed-by: Ralph Campbell
---
v2:
- Fix error unwind of mmgrab (Jerome)
- Use hmm local instead of 2nd container_of (Jerome)
---
mm/hmm.c | 80
1 file changed, 29 insertions(+), 51 deletions(-)
diff --git a/mm/hmm.c b/mm/hmm.c
index cc7c26fda3300e..dc30edad9a8a02 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -40,16 +40,6 @@
#if IS_ENABLED(CONFIG_HMM_MIRROR)
static const struct mmu_notifier_ops hmm_mmu_notifier_ops;
-static inline struct hmm *mm_get_hmm(struct mm_struct *mm)
-{
- struct hmm *hmm = READ_ONCE(mm->hmm);
-
- if (hmm && kref_get_unless_zero(>kref))
- return hmm;
-
- return NULL;
-}
-
/**
* hmm_get_or_create - register HMM against an mm (HMM internal)
*
@@ -64,11 +54,20 @@ static inline struct hmm *mm_get_hmm(struct mm_struct *mm)
*/
static struct hmm *hmm_get_or_create(struct mm_struct *mm)
{
- struct hmm *hmm = mm_get_hmm(mm);
- bool cleanup = false;
+ struct hmm *hmm;
- if (hmm)
- return hmm;
+ lockdep_assert_held_exclusive(>mmap_sem);
+
+ if (mm->hmm) {
+ if (kref_get_unless_zero(>hmm->kref))
+ return mm->hmm;
+ /*
+* The hmm is being freed by some other CPU and is pending a
+* RCU grace period, but this CPU can NULL now it since we
+* have the mmap_sem.
+*/
+ mm->hmm = NULL;
+ }
hmm = kmalloc(sizeof(*hmm), GFP_KERNEL);
if (!hmm)
@@ -83,57 +82,36 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
hmm->notifiers = 0;
hmm->dead = false;
hmm->mm = mm;
- mmgrab(hmm->mm);
-
- spin_lock(>page_table_lock);
- if (!mm->hmm)
- mm->hmm = hmm;
- else
- cleanup = true;
- spin_unlock(>page_table_lock);
- if (cleanup)
- goto error;
-
- /*
-* We should only get here if hold the mmap_sem in write mode ie on
-* registration of first mirror through hmm_mirror_register()
-*/
hmm->mmu_notifier.ops = _mmu_notifier_ops;
- if (__mmu_notifier_register(>mmu_notifier, mm))
- goto error_mm;
+ if (__mmu_notifier_register(>mmu_notifier, mm)) {
+ kfree(hmm);
+ return NULL;
+ }
+ mmgrab(hmm->mm);
+ mm->hmm = hmm;
return hmm;
-
-error_mm:
- spin_lock(>page_table_lock);
- if (mm->hmm == hmm)
- mm->hmm = NULL;
- spin_unlock(>page_table_lock);
-error:
- mmdrop(hmm->mm);
- kfree(hmm);
- return NULL;
}
static void hmm_free_rcu(struct rcu_head *rcu)
{
- kfree(container_of(rcu, struct hmm, rcu));
+ struct hmm *hmm = container_of(rcu, struct hmm, rcu);
+
+ down_write(>mm->mmap_sem);
+ if (hmm->mm->hmm == hmm)
+ hmm->mm->hmm = NULL;
+ up_write(>mm->mmap_sem);
+ mmdrop(hmm->mm);
+
+ kfree(hmm);
}
static void hmm_free(struct kref *kref)
{
struct hmm *hmm = container_of(kref, struct hmm, kref);
- struct mm_struct *mm = hmm->mm;
-
- mmu_notifier_unregister_no_release(>mmu_notifier, mm);
- spin_lock(>page_table_lock);
- if (mm->hmm == hmm)
- mm->hmm = NULL;
- spin_unlock(>page_table_lock);
-
- mmdrop(hmm->mm);
+ mmu_notifier_unregister_no_release(>mmu_notifier, hmm->mm);
mmu_notifier_call_srcu(>rcu, hmm_free_rcu);
}
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 11/11] mm/hmm: Remove confusing comment and logic from hmm_release
On Fri, Jun 07, 2019 at 02:37:07PM -0700, Ralph Campbell wrote: > > On 6/6/19 11:44 AM, Jason Gunthorpe wrote: > > From: Jason Gunthorpe > > > > hmm_release() is called exactly once per hmm. ops->release() cannot > > accidentally trigger any action that would recurse back onto > > hmm->mirrors_sem. > > > > This fixes a use after-free race of the form: > > > > CPU0 CPU1 > > hmm_release() > > up_write(>mirrors_sem); > > hmm_mirror_unregister(mirror) > >down_write(>mirrors_sem); > >up_write(>mirrors_sem); > >kfree(mirror) > > mirror->ops->release(mirror) > > > > The only user we have today for ops->release is an empty function, so this > > is unambiguously safe. > > > > As a consequence of plugging this race drivers are not allowed to > > register/unregister mirrors from within a release op. > > > > Signed-off-by: Jason Gunthorpe > > I agree with the analysis above but I'm not sure that release() will > always be an empty function. It might be more efficient to write back > all data migrated to a device "in one pass" instead of relying > on unmap_vmas() calling hmm_start_range_invalidate() per VMA. Sure, but it should not be allowed to recurse back to hmm_mirror_unregister. > I think the bigger issue is potential deadlocks while calling > sync_cpu_device_pagetables() and tasks calling hmm_mirror_unregister(): > > Say you have three threads: > - Thread A is in try_to_unmap(), either without holding mmap_sem or with > mmap_sem held for read. > - Thread B has some unrelated driver calling hmm_mirror_unregister(). > This doesn't require mmap_sem. > - Thread C is about to call migrate_vma(). > > Thread AThread B Thread C > try_to_unmaphmm_mirror_unregistermigrate_vma > hmm_invalidate_range_start > down_read(mirrors_sem) > down_write(mirrors_sem) > // Blocked on A > device_lock > device_lock > // Blocked on C > migrate_vma() > hmm_invalidate_range_s > down_read(mirrors_sem) > // Blocked on B > // Deadlock Oh... you know I didn't know this about rwsems in linux that they have a fairness policy for writes to block future reads.. Still, at least as things are designed, the driver cannot hold a lock it obtains under sync_cpu_device_pagetables() and nest other things in that lock. It certainly can't recurse back into any mmu notifiers while holding that lock. (as you point out) The lock in sync_cpu_device_pagetables() needs to be very narrowly focused on updating device state only. So, my first reaction is that the driver in thread C is wrong, and needs a different locking scheme. I think you'd have to make a really good case that there is no alternative for a driver.. > Perhaps we should consider using SRCU for walking the mirror->list? It means the driver has to deal with races like in this patch description. At that point there is almost no reason to insert hmm here, just use mmu notifiers directly. Drivers won't get this right, it is too hard. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 01/11] mm/hmm: fix use after free with struct hmm in the mmu notifiers
On 6/7/19 5:34 AM, Jason Gunthorpe wrote:
> On Thu, Jun 06, 2019 at 07:29:08PM -0700, John Hubbard wrote:
>> On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
>>> From: Jason Gunthorpe
>> ...
>>> @@ -153,10 +158,14 @@ void hmm_mm_destroy(struct mm_struct *mm)
>>>
>>> static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
>>> {
>>> - struct hmm *hmm = mm_get_hmm(mm);
>>> + struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
>>> struct hmm_mirror *mirror;
>>> struct hmm_range *range;
>>>
>>> + /* hmm is in progress to free */
>>
>> Well, sometimes, yes. :)
>
> It think it is in all cases actually.. The only way we see a 0 kref
> and still reach this code path is if another thread has alreay setup
> the hmm_free in the call_srcu..
>
>> Maybe this wording is clearer (if we need any comment at all):
>
> I always find this hard.. This is a very standard pattern when working
> with RCU - however in my experience few people actually know the RCU
> patterns, and missing the _unless_zero is a common bug I find when
> looking at code.
>
> This is mm/ so I can drop it, what do you think?
>
I forgot to respond to this section, so catching up now:
I think we're talking about slightly different things. I was just
noting that the comment above the "if" statement was only accurate
if the branch is taken, which is why I recommended this combination
of comment and code:
/* Bail out if hmm is in the process of being freed */
if (!kref_get_unless_zero(>kref))
return;
As for the actual _unless_zero part, I think that's good to have.
And it's a good reminder if nothing else, even in mm/ code.
thanks,
--
John Hubbard
NVIDIA
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] mm/hmm: pass mmu_notifier_range to sync_cpu_device_pagetables
On Fri, Jun 07, 2019 at 05:14:52PM -0700, Ralph Campbell wrote: > HMM defines its own struct hmm_update which is passed to the > sync_cpu_device_pagetables() callback function. This is > sufficient when the only action is to invalidate. However, > a device may want to know the reason for the invalidation and > be able to see the new permissions on a range, update device access > rights or range statistics. Since sync_cpu_device_pagetables() > can be called from try_to_unmap(), the mmap_sem may not be held > and find_vma() is not safe to be called. > Pass the struct mmu_notifier_range to sync_cpu_device_pagetables() > to allow the full invalidation information to be used. > > Signed-off-by: Ralph Campbell > --- > > I'm sending this out now since we are updating many of the HMM APIs > and I think it will be useful. I agree with CH that struct hmm_update seems particularly pointless and we really should just use mmu_notifier_range directly. We need to find out from the DRM folks if we can merge this kind of stuff through hmm.git and then resolve any conflicts that might arise in DRM tree or in nouveau tree? But I would like to see this patch go in this cycle, thanks Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm: add fallback override/firmware EDID modes workaround
On Sat, 2019-06-08 at 13:10 +0800, Paul Wise wrote: > I've tested these two patches on top of Linux v5.2-rc3 and the EDID > override works correctly on an Intel Ironlake GPU with a monitor that > lost its EDID a while ago. While testing I noticed a couple of things: While everything the GUI is the correct resolution, GNOME is unable to identify the monitor vendor or model. This is a regression from the previous edid override functionality. It looks like this is because the edid file in /sys is not populated with the EDID override data. I got a crash due to null pointer dereference at one point, I'll try to track down when this happens. -- bye, pabs https://bonedaddy.net/pabs3/ signature.asc Description: This is a digitally signed message part ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 07/11] mm/hmm: Use lockdep instead of comments
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
From: Jason Gunthorpe
So we can check locking at runtime.
Signed-off-by: Jason Gunthorpe
Reviewed-by: Jérôme Glisse
Reviewed-by: Ralph Campbell
---
v2
- Fix missing & in lockdeps (Jason)
---
mm/hmm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/hmm.c b/mm/hmm.c
index f67ba32983d9f1..c702cd72651b53 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -254,11 +254,11 @@ static const struct mmu_notifier_ops hmm_mmu_notifier_ops
= {
*
* To start mirroring a process address space, the device driver must register
* an HMM mirror struct.
- *
- * THE mm->mmap_sem MUST BE HELD IN WRITE MODE !
*/
int hmm_mirror_register(struct hmm_mirror *mirror, struct mm_struct *mm)
{
+ lockdep_assert_held_exclusive(>mmap_sem);
+
/* Sanity check */
if (!mm || !mirror || !mirror->ops)
return -EINVAL;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 05/11] mm/hmm: Remove duplicate condition test before wait_event_timeout
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
From: Jason Gunthorpe
The wait_event_timeout macro already tests the condition as its first
action, so there is no reason to open code another version of this, all
that does is skip the might_sleep() debugging in common cases, which is
not helpful.
Further, based on prior patches, we can no simplify the required condition
test:
- If range is valid memory then so is range->hmm
- If hmm_release() has run then range->valid is set to false
at the same time as dead, so no reason to check both.
- A valid hmm has a valid hmm->mm.
Also, add the READ_ONCE for range->valid as there is no lock held here.
Signed-off-by: Jason Gunthorpe
Reviewed-by: Jérôme Glisse
---
include/linux/hmm.h | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 4ee3acabe5ed22..2ab35b40992b24 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -218,17 +218,9 @@ static inline unsigned long hmm_range_page_size(const
struct hmm_range *range)
static inline bool hmm_range_wait_until_valid(struct hmm_range *range,
unsigned long timeout)
{
- /* Check if mm is dead ? */
- if (range->hmm == NULL || range->hmm->dead || range->hmm->mm == NULL) {
- range->valid = false;
- return false;
- }
- if (range->valid)
- return true;
- wait_event_timeout(range->hmm->wq, range->valid || range->hmm->dead,
+ wait_event_timeout(range->hmm->wq, range->valid,
msecs_to_jiffies(timeout));
- /* Return current valid status just in case we get lucky */
- return range->valid;
+ return READ_ONCE(range->valid);
}
/*
Since we are simplifying things, perhaps we should consider merging
hmm_range_wait_until_valid() info hmm_range_register() and
removing hmm_range_wait_until_valid() since the pattern
is to always call the two together.
In any case, this looks OK to me so you can add
Reviewed-by: Ralph Campbell
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 12/11] mm/hmm: Fix error flows in hmm_invalidate_range_start
If the trylock on the hmm->mirrors_sem fails the function will return
without decrementing the notifiers that were previously incremented. Since
the caller will not call invalidate_range_end() on EAGAIN this will result
in notifiers becoming permanently incremented and deadlock.
If the sync_cpu_device_pagetables() required blocking the function will
not return EAGAIN even though the device continues to touch the
pages. This is a violation of the mmu notifier contract.
Switch, and rename, the ranges_lock to a spin lock so we can reliably
obtain it without blocking during error unwind.
The error unwind is necessary since the notifiers count must be held
incremented across the call to sync_cpu_device_pagetables() as we cannot
allow the range to become marked valid by a parallel
invalidate_start/end() pair while doing sync_cpu_device_pagetables().
Signed-off-by: Jason Gunthorpe
---
include/linux/hmm.h | 2 +-
mm/hmm.c| 77 +++--
2 files changed, 48 insertions(+), 31 deletions(-)
I almost lost this patch - it is part of the series, hasn't been
posted before, and wasn't sent with the rest, sorry.
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index bf013e96525771..0fa8ea34ccef6d 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -86,7 +86,7 @@
struct hmm {
struct mm_struct*mm;
struct kref kref;
- struct mutexlock;
+ spinlock_t ranges_lock;
struct list_headranges;
struct list_headmirrors;
struct mmu_notifier mmu_notifier;
diff --git a/mm/hmm.c b/mm/hmm.c
index 4215edf737ef5b..10103a24e9b7b3 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -68,7 +68,7 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
init_rwsem(>mirrors_sem);
hmm->mmu_notifier.ops = NULL;
INIT_LIST_HEAD(>ranges);
- mutex_init(>lock);
+ spin_lock_init(>ranges_lock);
kref_init(>kref);
hmm->notifiers = 0;
hmm->mm = mm;
@@ -114,18 +114,19 @@ static void hmm_release(struct mmu_notifier *mn, struct
mm_struct *mm)
{
struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
struct hmm_mirror *mirror;
+ unsigned long flags;
/* Bail out if hmm is in the process of being freed */
if (!kref_get_unless_zero(>kref))
return;
- mutex_lock(>lock);
+ spin_lock_irqsave(>ranges_lock, flags);
/*
* Since hmm_range_register() holds the mmget() lock hmm_release() is
* prevented as long as a range exists.
*/
WARN_ON(!list_empty(>ranges));
- mutex_unlock(>lock);
+ spin_unlock_irqrestore(>ranges_lock, flags);
down_read(>mirrors_sem);
list_for_each_entry(mirror, >mirrors, list) {
@@ -141,6 +142,23 @@ static void hmm_release(struct mmu_notifier *mn, struct
mm_struct *mm)
hmm_put(hmm);
}
+static void notifiers_decrement(struct hmm *hmm)
+{
+ lockdep_assert_held(>ranges_lock);
+
+ hmm->notifiers--;
+ if (!hmm->notifiers) {
+ struct hmm_range *range;
+
+ list_for_each_entry(range, >ranges, list) {
+ if (range->valid)
+ continue;
+ range->valid = true;
+ }
+ wake_up_all(>wq);
+ }
+}
+
static int hmm_invalidate_range_start(struct mmu_notifier *mn,
const struct mmu_notifier_range *nrange)
{
@@ -148,6 +166,7 @@ static int hmm_invalidate_range_start(struct mmu_notifier
*mn,
struct hmm_mirror *mirror;
struct hmm_update update;
struct hmm_range *range;
+ unsigned long flags;
int ret = 0;
if (!kref_get_unless_zero(>kref))
@@ -158,12 +177,7 @@ static int hmm_invalidate_range_start(struct mmu_notifier
*mn,
update.event = HMM_UPDATE_INVALIDATE;
update.blockable = mmu_notifier_range_blockable(nrange);
- if (mmu_notifier_range_blockable(nrange))
- mutex_lock(>lock);
- else if (!mutex_trylock(>lock)) {
- ret = -EAGAIN;
- goto out;
- }
+ spin_lock_irqsave(>ranges_lock, flags);
hmm->notifiers++;
list_for_each_entry(range, >ranges, list) {
if (update.end < range->start || update.start >= range->end)
@@ -171,7 +185,7 @@ static int hmm_invalidate_range_start(struct mmu_notifier
*mn,
range->valid = false;
}
- mutex_unlock(>lock);
+ spin_unlock_irqrestore(>ranges_lock, flags);
if (mmu_notifier_range_blockable(nrange))
down_read(>mirrors_sem);
@@ -179,16 +193,26 @@ static int hmm_invalidate_range_start(struct mmu_notifier
*mn,
ret = -EAGAIN;
goto out;
}
+
list_for_each_entry(mirror, >mirrors, list) {
- int ret;
+
Re: [PATCH v2 hmm 03/11] mm/hmm: Hold a mmgrab from hmm to mm
On Fri, Jun 07, 2019 at 11:41:20AM -0700, Ralph Campbell wrote: > > On 6/6/19 11:44 AM, Jason Gunthorpe wrote: > > From: Jason Gunthorpe > > > > So long a a struct hmm pointer exists, so should the struct mm it is > > s/a a/as a/ Got it, thanks Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 05/11] mm/hmm: Remove duplicate condition test before wait_event_timeout
On Fri, Jun 07, 2019 at 12:01:45PM -0700, Ralph Campbell wrote:
>
> On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
> > From: Jason Gunthorpe
> >
> > The wait_event_timeout macro already tests the condition as its first
> > action, so there is no reason to open code another version of this, all
> > that does is skip the might_sleep() debugging in common cases, which is
> > not helpful.
> >
> > Further, based on prior patches, we can no simplify the required condition
> > test:
> > - If range is valid memory then so is range->hmm
> > - If hmm_release() has run then range->valid is set to false
> > at the same time as dead, so no reason to check both.
> > - A valid hmm has a valid hmm->mm.
> >
> > Also, add the READ_ONCE for range->valid as there is no lock held here.
> >
> > Signed-off-by: Jason Gunthorpe
> > Reviewed-by: Jérôme Glisse
> > include/linux/hmm.h | 12 ++--
> > 1 file changed, 2 insertions(+), 10 deletions(-)
> >
> > diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> > index 4ee3acabe5ed22..2ab35b40992b24 100644
> > +++ b/include/linux/hmm.h
> > @@ -218,17 +218,9 @@ static inline unsigned long hmm_range_page_size(const
> > struct hmm_range *range)
> > static inline bool hmm_range_wait_until_valid(struct hmm_range *range,
> > unsigned long timeout)
> > {
> > - /* Check if mm is dead ? */
> > - if (range->hmm == NULL || range->hmm->dead || range->hmm->mm == NULL) {
> > - range->valid = false;
> > - return false;
> > - }
> > - if (range->valid)
> > - return true;
> > - wait_event_timeout(range->hmm->wq, range->valid || range->hmm->dead,
> > + wait_event_timeout(range->hmm->wq, range->valid,
> >msecs_to_jiffies(timeout));
> > - /* Return current valid status just in case we get lucky */
> > - return range->valid;
> > + return READ_ONCE(range->valid);
> > }
> > /*
> >
>
> Since we are simplifying things, perhaps we should consider merging
> hmm_range_wait_until_valid() info hmm_range_register() and
> removing hmm_range_wait_until_valid() since the pattern
> is to always call the two together.
? the hmm.rst shows the hmm_range_wait_until_valid being called in the
(ret == -EAGAIN) path. It is confusing because it should really just
have the again label moved up above hmm_range_wait_until_valid() as
even if we get the driver lock it could still be a long wait for the
colliding invalidation to clear.
What I want to get to is a pattern like this:
pagefault():
hmm_range_register();
again:
/* On the slow path, if we appear to be live locked then we get
the write side of mmap_sem which will break the live lock,
otherwise this gets the read lock */
if (hmm_range_start_and_lock())
goto err;
lockdep_assert_held(range->mm->mmap_sem);
// Optional: Avoid useless expensive work
if (hmm_range_needs_retry())
goto again;
hmm_range_(touch vmas)
take_lock(driver->update);
if (hmm_range_end() {
release_lock(driver->update);
goto again;
}
// Finish driver updates
release_lock(driver->update);
// Releases mmap_sem
hmm_range_unregister_and_unlock();
What do you think?
Is it clear?
Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 01/11] mm/hmm: fix use after free with struct hmm in the mmu notifiers
On Sat, Jun 08, 2019 at 01:49:48AM -0700, Christoph Hellwig wrote: > I still think sruct hmm should die. We already have a structure used > for additional information for drivers having crazly tight integration > into the VM, and it is called struct mmu_notifier_mm. We really need > to reuse that intead of duplicating it badly. Probably. But at least in ODP we needed something very similar to 'struct hmm' to make our mmu notifier implementation work. The mmu notifier api really lends itself to having a per-mm structure in the driver to hold the 'struct mmu_notifier'.. I think I see other drivers are doing things like assuming that there is only one mm in their world (despite being FD based, so this is not really guarenteed) So, my first attempt would be an api something like: priv = mmu_notififer_attach_mm(ops, current->mm, sizeof(my_priv)) mmu_notifier_detach_mm(priv); ops->invalidate_start(struct mmu_notififer *mn): struct p *priv = mmu_notifier_priv(mn); Such that - There is only one priv per mm - All the srcu stuff is handled inside mmu notifier - It is reference counted, so ops can be attached multiple times to the same mm Then odp's per_mm, and struct hmm (if we keep it at all) is simply a 'priv' in the above. I was thinking of looking at this stuff next, once this series is done. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/nouveau/svm: Convert to use hmm_range_fault()
Hi Jason,
On Tue, May 21, 2019 at 12:27 AM Souptick Joarder wrote:
>
> Convert to use hmm_range_fault().
>
> Signed-off-by: Souptick Joarder
Would you like to take it through your new hmm tree or do I
need to resend it ?
> ---
> drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c
> b/drivers/gpu/drm/nouveau/nouveau_svm.c
> index 93ed43c..8d56bd6 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> @@ -649,7 +649,7 @@ struct nouveau_svmm {
> range.values = nouveau_svm_pfn_values;
> range.pfn_shift = NVIF_VMM_PFNMAP_V0_ADDR_SHIFT;
> again:
> - ret = hmm_vma_fault(, true);
> + ret = hmm_range_fault(, true);
> if (ret == 0) {
> mutex_lock(>mutex);
> if (!hmm_vma_range_done()) {
> --
> 1.9.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm: add fallback override/firmware EDID modes workaround
On Fri, 2019-06-07 at 17:10 +0200, Daniel Vetter wrote: > As discussed on irc, we need tested-by here from the reporters since > there's way too many losing and frustrangingly few winning moves here. I'm building it now, hopefully will be done today. -- bye, pabs https://bonedaddy.net/pabs3/ signature.asc Description: This is a digitally signed message part ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] gpu: host1x: Increase maximum DMA segment size
05.06.2019 11:46, Thierry Reding пишет:
> From: Thierry Reding
>
> Recent versions of the DMA API debug code have started to warn about
> violations of the maximum DMA segment size. This is because the segment
> size defaults to 64 KiB, which can easily be exceeded in large buffer
> allocations such as used in DRM/KMS for framebuffers.
>
> Technically the Tegra SMMU and ARM SMMU don't have a maximum segment
> size (they map individual pages irrespective of whether they are
> contiguous or not), so the choice of 4 MiB is a bit arbitrary here. The
> maximum segment size is a 32-bit unsigned integer, though, so we can't
> set it to the correct maximum size, which would be the size of the
> aperture.
>
> Signed-off-by: Thierry Reding
> ---
> drivers/gpu/host1x/bus.c | 3 +++
> include/linux/host1x.h | 2 ++
> 2 files changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
> index 9797ccb0a073..6387302c1245 100644
> --- a/drivers/gpu/host1x/bus.c
> +++ b/drivers/gpu/host1x/bus.c
> @@ -414,6 +414,9 @@ static int host1x_device_add(struct host1x *host1x,
>
> of_dma_configure(>dev, host1x->dev->of_node, true);
>
> + device->dev.dma_parms = >dma_parms;
> + dma_set_max_seg_size(>dev, SZ_4M);
> +
> err = host1x_device_parse_dt(device, driver);
> if (err < 0) {
> kfree(device);
> diff --git a/include/linux/host1x.h b/include/linux/host1x.h
> index cfff30b9a62e..e6eea45e1154 100644
> --- a/include/linux/host1x.h
> +++ b/include/linux/host1x.h
> @@ -297,6 +297,8 @@ struct host1x_device {
> struct list_head clients;
>
> bool registered;
> +
> + struct device_dma_parameters dma_parms;
> };
>
> static inline struct host1x_device *to_host1x_device(struct device *dev)
>
Just a very minor nit:
It may be worthwhile to include "dma-mapping.h" directly for consistency
here as well since nothing includes it directly. I noticed a build
breakage of the grate-driver's kernel on a rebase with "git --exec
'make..'" cause we have some of "iommu/iova" headers that are moved
around in the intermediate patches.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 12/11] mm/hmm: Fix error flows in hmm_invalidate_range_start
On 6/7/19 9:05 AM, Jason Gunthorpe wrote:
If the trylock on the hmm->mirrors_sem fails the function will return
without decrementing the notifiers that were previously incremented. Since
the caller will not call invalidate_range_end() on EAGAIN this will result
in notifiers becoming permanently incremented and deadlock.
If the sync_cpu_device_pagetables() required blocking the function will
not return EAGAIN even though the device continues to touch the
pages. This is a violation of the mmu notifier contract.
Switch, and rename, the ranges_lock to a spin lock so we can reliably
obtain it without blocking during error unwind.
The error unwind is necessary since the notifiers count must be held
incremented across the call to sync_cpu_device_pagetables() as we cannot
allow the range to become marked valid by a parallel
invalidate_start/end() pair while doing sync_cpu_device_pagetables().
Signed-off-by: Jason Gunthorpe
Reviewed-by: Ralph Campbell
---
include/linux/hmm.h | 2 +-
mm/hmm.c| 77 +++--
2 files changed, 48 insertions(+), 31 deletions(-)
I almost lost this patch - it is part of the series, hasn't been
posted before, and wasn't sent with the rest, sorry.
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index bf013e96525771..0fa8ea34ccef6d 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -86,7 +86,7 @@
struct hmm {
struct mm_struct*mm;
struct kref kref;
- struct mutexlock;
+ spinlock_t ranges_lock;
struct list_headranges;
struct list_headmirrors;
struct mmu_notifier mmu_notifier;
diff --git a/mm/hmm.c b/mm/hmm.c
index 4215edf737ef5b..10103a24e9b7b3 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -68,7 +68,7 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
init_rwsem(>mirrors_sem);
hmm->mmu_notifier.ops = NULL;
INIT_LIST_HEAD(>ranges);
- mutex_init(>lock);
+ spin_lock_init(>ranges_lock);
kref_init(>kref);
hmm->notifiers = 0;
hmm->mm = mm;
@@ -114,18 +114,19 @@ static void hmm_release(struct mmu_notifier *mn, struct
mm_struct *mm)
{
struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
struct hmm_mirror *mirror;
+ unsigned long flags;
/* Bail out if hmm is in the process of being freed */
if (!kref_get_unless_zero(>kref))
return;
- mutex_lock(>lock);
+ spin_lock_irqsave(>ranges_lock, flags);
/*
* Since hmm_range_register() holds the mmget() lock hmm_release() is
* prevented as long as a range exists.
*/
WARN_ON(!list_empty(>ranges));
- mutex_unlock(>lock);
+ spin_unlock_irqrestore(>ranges_lock, flags);
down_read(>mirrors_sem);
list_for_each_entry(mirror, >mirrors, list) {
@@ -141,6 +142,23 @@ static void hmm_release(struct mmu_notifier *mn, struct
mm_struct *mm)
hmm_put(hmm);
}
+static void notifiers_decrement(struct hmm *hmm)
+{
+ lockdep_assert_held(>ranges_lock);
+
+ hmm->notifiers--;
+ if (!hmm->notifiers) {
+ struct hmm_range *range;
+
+ list_for_each_entry(range, >ranges, list) {
+ if (range->valid)
+ continue;
+ range->valid = true;
+ }
This just effectively sets all ranges to valid.
I'm not sure that is best.
Shouldn't hmm_range_register() start with range.valid = true and
then hmm_invalidate_range_start() set affected ranges to false?
Then this becomes just wake_up_all() if --notifiers == 0 and
hmm_range_wait_until_valid() should wait for notifiers == 0.
Otherwise, range.valid doesn't really mean it's valid.
+ wake_up_all(>wq);
+ }
+}
+
static int hmm_invalidate_range_start(struct mmu_notifier *mn,
const struct mmu_notifier_range *nrange)
{
@@ -148,6 +166,7 @@ static int hmm_invalidate_range_start(struct mmu_notifier
*mn,
struct hmm_mirror *mirror;
struct hmm_update update;
struct hmm_range *range;
+ unsigned long flags;
int ret = 0;
if (!kref_get_unless_zero(>kref))
@@ -158,12 +177,7 @@ static int hmm_invalidate_range_start(struct mmu_notifier
*mn,
update.event = HMM_UPDATE_INVALIDATE;
update.blockable = mmu_notifier_range_blockable(nrange);
- if (mmu_notifier_range_blockable(nrange))
- mutex_lock(>lock);
- else if (!mutex_trylock(>lock)) {
- ret = -EAGAIN;
- goto out;
- }
+ spin_lock_irqsave(>ranges_lock, flags);
hmm->notifiers++;
list_for_each_entry(range, >ranges, list) {
if (update.end < range->start || update.start >= range->end)
@@ -171,7 +185,7 @@ static int hmm_invalidate_range_start(struct mmu_notifier
*mn,
Re: [PATCH v2 hmm 02/11] mm/hmm: Use hmm_mirror not mm as an argument for hmm_range_register
On 6/7/19 11:24 AM, Ralph Campbell wrote:
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
From: Jason Gunthorpe
Ralph observes that hmm_range_register() can only be called by a driver
while a mirror is registered. Make this clear in the API by passing in
the
mirror structure as a parameter.
This also simplifies understanding the lifetime model for struct hmm, as
the hmm pointer must be valid as part of a registered mirror so all we
need in hmm_register_range() is a simple kref_get.
Suggested-by: Ralph Campbell
Signed-off-by: Jason Gunthorpe
You might CC Ben for the nouveau part.
CC: Ben Skeggs
Reviewed-by: Ralph Campbell
---
v2
- Include the oneline patch to nouveau_svm.c
---
drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +-
include/linux/hmm.h | 7 ---
mm/hmm.c | 15 ++-
3 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c
b/drivers/gpu/drm/nouveau/nouveau_svm.c
index 93ed43c413f0bb..8c92374afcf227 100644
--- a/drivers/gpu/drm/nouveau/nouveau_svm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
@@ -649,7 +649,7 @@ nouveau_svm_fault(struct nvif_notify *notify)
range.values = nouveau_svm_pfn_values;
range.pfn_shift = NVIF_VMM_PFNMAP_V0_ADDR_SHIFT;
again:
- ret = hmm_vma_fault(, true);
+ ret = hmm_vma_fault(>mirror, , true);
if (ret == 0) {
mutex_lock(>mutex);
if (!hmm_vma_range_done()) {
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 688c5ca7068795..2d519797cb134a 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -505,7 +505,7 @@ static inline bool hmm_mirror_mm_is_alive(struct
hmm_mirror *mirror)
* Please see Documentation/vm/hmm.rst for how to use the range API.
*/
int hmm_range_register(struct hmm_range *range,
- struct mm_struct *mm,
+ struct hmm_mirror *mirror,
unsigned long start,
unsigned long end,
unsigned page_shift);
@@ -541,7 +541,8 @@ static inline bool hmm_vma_range_done(struct
hmm_range *range)
}
/* This is a temporary helper to avoid merge conflict between trees. */
-static inline int hmm_vma_fault(struct hmm_range *range, bool block)
+static inline int hmm_vma_fault(struct hmm_mirror *mirror,
+ struct hmm_range *range, bool block)
{
long ret;
@@ -554,7 +555,7 @@ static inline int hmm_vma_fault(struct hmm_range
*range, bool block)
range->default_flags = 0;
range->pfn_flags_mask = -1UL;
- ret = hmm_range_register(range, range->vma->vm_mm,
+ ret = hmm_range_register(range, mirror,
range->start, range->end,
PAGE_SHIFT);
if (ret)
diff --git a/mm/hmm.c b/mm/hmm.c
index 547002f56a163d..8796447299023c 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -925,13 +925,13 @@ static void hmm_pfns_clear(struct hmm_range *range,
* Track updates to the CPU page table see include/linux/hmm.h
*/
int hmm_range_register(struct hmm_range *range,
- struct mm_struct *mm,
+ struct hmm_mirror *mirror,
unsigned long start,
unsigned long end,
unsigned page_shift)
{
unsigned long mask = ((1UL << page_shift) - 1UL);
- struct hmm *hmm;
+ struct hmm *hmm = mirror->hmm;
range->valid = false;
range->hmm = NULL;
@@ -945,15 +945,12 @@ int hmm_range_register(struct hmm_range *range,
range->start = start;
range->end = end;
- hmm = hmm_get_or_create(mm);
- if (!hmm)
- return -EFAULT;
-
/* Check if hmm_mm_destroy() was call. */
- if (hmm->mm == NULL || hmm->dead) {
- hmm_put(hmm);
+ if (hmm->mm == NULL || hmm->dead)
return -EFAULT;
- }
+
+ range->hmm = hmm;
+ kref_get(>kref);
/* Initialize range to track CPU page table updates. */
mutex_lock(>lock);
I forgot to add that I think you can delete the duplicate
"range->hmm = hmm;"
here between the mutex_lock/unlock.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 hmm 05/11] mm/hmm: Remove duplicate condition test before wait_event_timeout
The wait_event_timeout macro already tests the condition as its first
action, so there is no reason to open code another version of this, all
that does is skip the might_sleep() debugging in common cases, which is
not helpful.
Further, based on prior patches, we can now simplify the required condition
test:
- If range is valid memory then so is range->hmm
- If hmm_release() has run then range->valid is set to false
at the same time as dead, so no reason to check both.
- A valid hmm has a valid hmm->mm.
Allowing the return value of wait_event_timeout() (along with its internal
barriers) to compute the result of the function.
Signed-off-by: Jason Gunthorpe
---
v3
- Simplify the wait_event_timeout to not check valid
---
include/linux/hmm.h | 13 ++---
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 1d97b6d62c5bcf..26e7c477490c4e 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -209,17 +209,8 @@ static inline unsigned long hmm_range_page_size(const
struct hmm_range *range)
static inline bool hmm_range_wait_until_valid(struct hmm_range *range,
unsigned long timeout)
{
- /* Check if mm is dead ? */
- if (range->hmm == NULL || range->hmm->dead || range->hmm->mm == NULL) {
- range->valid = false;
- return false;
- }
- if (range->valid)
- return true;
- wait_event_timeout(range->hmm->wq, range->valid || range->hmm->dead,
- msecs_to_jiffies(timeout));
- /* Return current valid status just in case we get lucky */
- return range->valid;
+ return wait_event_timeout(range->hmm->wq, range->valid,
+ msecs_to_jiffies(timeout)) != 0;
}
/*
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 06/11] mm/hmm: Hold on to the mmget for the lifetime of the range
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
From: Jason Gunthorpe
Range functions like hmm_range_snapshot() and hmm_range_fault() call
find_vma, which requires hodling the mmget() and the mmap_sem for the mm.
Make this simpler for the callers by holding the mmget() inside the range
for the lifetime of the range. Other functions that accept a range should
only be called if the range is registered.
This has the side effect of directly preventing hmm_release() from
happening while a range is registered. That means range->dead cannot be
false during the lifetime of the range, so remove dead and
hmm_mirror_mm_is_alive() entirely.
Signed-off-by: Jason Gunthorpe
Looks good to me.
Reviewed-by: Ralph Campbell
---
v2:
- Use Jerome's idea of just holding the mmget() for the range lifetime,
rework the patch to use that as as simplification to remove dead in
one step
---
include/linux/hmm.h | 26 --
mm/hmm.c| 28 ++--
2 files changed, 10 insertions(+), 44 deletions(-)
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 2ab35b40992b24..0e20566802967a 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -91,7 +91,6 @@
* @mirrors_sem: read/write semaphore protecting the mirrors list
* @wq: wait queue for user waiting on a range invalidation
* @notifiers: count of active mmu notifiers
- * @dead: is the mm dead ?
*/
struct hmm {
struct mm_struct*mm;
@@ -104,7 +103,6 @@ struct hmm {
wait_queue_head_t wq;
struct rcu_head rcu;
longnotifiers;
- booldead;
};
/*
@@ -469,30 +467,6 @@ struct hmm_mirror {
int hmm_mirror_register(struct hmm_mirror *mirror, struct mm_struct *mm);
void hmm_mirror_unregister(struct hmm_mirror *mirror);
-/*
- * hmm_mirror_mm_is_alive() - test if mm is still alive
- * @mirror: the HMM mm mirror for which we want to lock the mmap_sem
- * Return: false if the mm is dead, true otherwise
- *
- * This is an optimization, it will not always accurately return false if the
- * mm is dead; i.e., there can be false negatives (process is being killed but
- * HMM is not yet informed of that). It is only intended to be used to optimize
- * out cases where the driver is about to do something time consuming and it
- * would be better to skip it if the mm is dead.
- */
-static inline bool hmm_mirror_mm_is_alive(struct hmm_mirror *mirror)
-{
- struct mm_struct *mm;
-
- if (!mirror || !mirror->hmm)
- return false;
- mm = READ_ONCE(mirror->hmm->mm);
- if (mirror->hmm->dead || !mm)
- return false;
-
- return true;
-}
-
/*
* Please see Documentation/vm/hmm.rst for how to use the range API.
*/
diff --git a/mm/hmm.c b/mm/hmm.c
index dc30edad9a8a02..f67ba32983d9f1 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -80,7 +80,6 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
mutex_init(>lock);
kref_init(>kref);
hmm->notifiers = 0;
- hmm->dead = false;
hmm->mm = mm;
hmm->mmu_notifier.ops = _mmu_notifier_ops;
@@ -124,20 +123,17 @@ static void hmm_release(struct mmu_notifier *mn, struct
mm_struct *mm)
{
struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
struct hmm_mirror *mirror;
- struct hmm_range *range;
/* hmm is in progress to free */
if (!kref_get_unless_zero(>kref))
return;
- /* Report this HMM as dying. */
- hmm->dead = true;
-
- /* Wake-up everyone waiting on any range. */
mutex_lock(>lock);
- list_for_each_entry(range, >ranges, list)
- range->valid = false;
- wake_up_all(>wq);
+ /*
+* Since hmm_range_register() holds the mmget() lock hmm_release() is
+* prevented as long as a range exists.
+*/
+ WARN_ON(!list_empty(>ranges));
mutex_unlock(>lock);
down_write(>mirrors_sem);
@@ -909,8 +905,8 @@ int hmm_range_register(struct hmm_range *range,
range->start = start;
range->end = end;
- /* Check if hmm_mm_destroy() was call. */
- if (hmm->mm == NULL || hmm->dead)
+ /* Prevent hmm_release() from running while the range is valid */
+ if (!mmget_not_zero(hmm->mm))
return -EFAULT;
range->hmm = hmm;
@@ -955,6 +951,7 @@ void hmm_range_unregister(struct hmm_range *range)
/* Drop reference taken by hmm_range_register() */
range->valid = false;
+ mmput(hmm->mm);
hmm_put(hmm);
range->hmm = NULL;
}
@@ -982,10 +979,7 @@ long hmm_range_snapshot(struct hmm_range *range)
struct vm_area_struct *vma;
struct mm_walk mm_walk;
- /* Check if hmm_mm_destroy() was call. */
- if (hmm->mm == NULL || hmm->dead)
- return -EFAULT;
-
+ lockdep_assert_held(>mm->mmap_sem);
do {
/* If
Re: [PATCH v2 hmm 05/11] mm/hmm: Remove duplicate condition test before wait_event_timeout
On Fri, Jun 07, 2019 at 03:13:00PM -0700, Ralph Campbell wrote:
> > Do you see a reason why the find_vma() ever needs to be before the
> > 'again' in my above example? range.vma does not need to be set for
> > range_register.
>
> Yes, for the GPU case, there can be many faults in an event queue
> and the goal is to try to handle more than one page at a time.
> The vma is needed to limit the amount of coalescing and checking
> for pages that could be speculatively migrated or mapped.
I'd need to see an algorithm sketch to see what you are thinking..
But, I guess a driver would have figure out a list of what virtual
pages it wants to fault under the mmap sem (maybe use find_vam, etc),
then drop mmap_sem, and start processing those pages for mirroring
under the hmm side.
ie they are two seperate unrelated tasks.
I looked at the hmm.rst again, and that reference algorithm is already
showing that that upon retry the mmap_sem is released:
take_lock(driver->update);
if (!range.valid) {
release_lock(driver->update);
up_read(>mmap_sem);
goto again;
So a driver cannot assume that any VMA related work done under the
mmap before the hmm 'critical section' can carry into the hmm critical
section as the lock can be released upon retry and invalidate all that
data..
Forcing the hmm_range_start_and_lock() to acquire the mmap_sem is a
rough way to force the driver author to realize there are two locking
domains and lock protected information cannot cross between.
> OK, I understand.
> If you come up with a set of changes, I can try testing them.
Thanks, I make a sketch of the patch, I have to get back to it after
this series is done.
Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 09/11] mm/hmm: Poison hmm_range during unregister
On 6/6/19 11:44 AM, Jason Gunthorpe wrote: From: Jason Gunthorpe Trying to misuse a range outside its lifetime is a kernel bug. Use WARN_ON and poison bytes to detect this condition. Signed-off-by: Jason Gunthorpe Reviewed-by: Jérôme Glisse Reviewed-by: Ralph Campbell --- v2 - Keep range start/end valid after unregistration (Jerome) --- mm/hmm.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/mm/hmm.c b/mm/hmm.c index 6802de7080d172..c2fecb3ecb11e1 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -937,7 +937,7 @@ void hmm_range_unregister(struct hmm_range *range) struct hmm *hmm = range->hmm; /* Sanity check this really should not happen. */ - if (hmm == NULL || range->end <= range->start) + if (WARN_ON(range->end <= range->start)) return; WARN_ON() is definitely better than silent return but I wonder how useful it is since the caller shouldn't be modifying the hmm_range once it is registered. Other fields could be changed too... mutex_lock(>lock); @@ -948,7 +948,10 @@ void hmm_range_unregister(struct hmm_range *range) range->valid = false; mmput(hmm->mm); hmm_put(hmm); - range->hmm = NULL; + + /* The range is now invalid, leave it poisoned. */ + range->valid = false; + memset(>hmm, POISON_INUSE, sizeof(range->hmm)); } EXPORT_SYMBOL(hmm_range_unregister); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 01/11] mm/hmm: fix use after free with struct hmm in the mmu notifiers
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
From: Jason Gunthorpe
mmu_notifier_unregister_no_release() is not a fence and the mmu_notifier
system will continue to reference hmm->mn until the srcu grace period
expires.
Resulting in use after free races like this:
CPU0 CPU1
__mmu_notifier_invalidate_range_start()
srcu_read_lock
hlist_for_each ()
// mn == hmm->mn
hmm_mirror_unregister()
hmm_put()
hmm_free()
mmu_notifier_unregister_no_release()
hlist_del_init_rcu(hmm-mn->list)
mn->ops->invalidate_range_start(mn, range);
mm_get_hmm()
mm->hmm = NULL;
kfree(hmm)
mutex_lock(>lock);
Use SRCU to kfree the hmm memory so that the notifiers can rely on hmm
existing. Get the now-safe hmm struct through container_of and directly
check kref_get_unless_zero to lock it against free.
Signed-off-by: Jason Gunthorpe
You can add
Reviewed-by: Ralph Campbell
---
v2:
- Spell 'free' properly (Jerome/Ralph)
---
include/linux/hmm.h | 1 +
mm/hmm.c| 25 +++--
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 092f0234bfe917..688c5ca7068795 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -102,6 +102,7 @@ struct hmm {
struct mmu_notifier mmu_notifier;
struct rw_semaphore mirrors_sem;
wait_queue_head_t wq;
+ struct rcu_head rcu;
longnotifiers;
booldead;
};
diff --git a/mm/hmm.c b/mm/hmm.c
index 8e7403f081f44a..547002f56a163d 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -113,6 +113,11 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
return NULL;
}
+static void hmm_free_rcu(struct rcu_head *rcu)
+{
+ kfree(container_of(rcu, struct hmm, rcu));
+}
+
static void hmm_free(struct kref *kref)
{
struct hmm *hmm = container_of(kref, struct hmm, kref);
@@ -125,7 +130,7 @@ static void hmm_free(struct kref *kref)
mm->hmm = NULL;
spin_unlock(>page_table_lock);
- kfree(hmm);
+ mmu_notifier_call_srcu(>rcu, hmm_free_rcu);
}
static inline void hmm_put(struct hmm *hmm)
@@ -153,10 +158,14 @@ void hmm_mm_destroy(struct mm_struct *mm)
static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
{
- struct hmm *hmm = mm_get_hmm(mm);
+ struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
struct hmm_mirror *mirror;
struct hmm_range *range;
+ /* hmm is in progress to free */
+ if (!kref_get_unless_zero(>kref))
+ return;
+
/* Report this HMM as dying. */
hmm->dead = true;
@@ -194,13 +203,15 @@ static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
static int hmm_invalidate_range_start(struct mmu_notifier *mn,
const struct mmu_notifier_range *nrange)
{
- struct hmm *hmm = mm_get_hmm(nrange->mm);
+ struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
struct hmm_mirror *mirror;
struct hmm_update update;
struct hmm_range *range;
int ret = 0;
- VM_BUG_ON(!hmm);
+ /* hmm is in progress to free */
+ if (!kref_get_unless_zero(>kref))
+ return 0;
update.start = nrange->start;
update.end = nrange->end;
@@ -245,9 +256,11 @@ static int hmm_invalidate_range_start(struct mmu_notifier
*mn,
static void hmm_invalidate_range_end(struct mmu_notifier *mn,
const struct mmu_notifier_range *nrange)
{
- struct hmm *hmm = mm_get_hmm(nrange->mm);
+ struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
- VM_BUG_ON(!hmm);
+ /* hmm is in progress to free */
+ if (!kref_get_unless_zero(>kref))
+ return;
mutex_lock(>lock);
hmm->notifiers--;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [2/2] drm/panel: support for AUO kd101n80-45na wuxga dsi video mode panel
Hi Jitao.
Thanks for another panel driver.
The comments for the panel-boe-tv101wum-nl6 diver to extent
applies to this driver too.
Please address these and I will do a proper review of the next version.
I notice that error handlign is a little bit less in this driver.
consier what approch to use and see if you can align between these
drivers, and if in doubt use panel-simple as your role model.
Sam
On Sat, Jun 08, 2019 at 07:23:42PM +0800, Jitao Shi wrote:
> Add driver for AUO kd101n80-45na panel.
> This panel supports the resolution 1200x1920, dsi video mode
> and 4 lanes.
>
> Signed-off-by: Jitao Shi
> ---
> drivers/gpu/drm/panel/Kconfig | 10 +
> drivers/gpu/drm/panel/Makefile| 1 +
> .../gpu/drm/panel/panel-auo-kd101n80-45na.c | 352 ++
> 3 files changed, 363 insertions(+)
> create mode 100644 drivers/gpu/drm/panel/panel-auo-kd101n80-45na.c
>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index e36dbb4df867..f5cd5af9ce42 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -272,4 +272,14 @@ config DRM_PANEL_TRULY_NT35597_WQXGA
> help
> Say Y here if you want to enable support for Truly NT35597 WQXGA Dual
> DSI
> Video Mode panel
> +
> +config DRM_PANEL_AUO_KD101N80_45NA
> + tristate "AUO KD101N80_45NA 1200x1920 panel"
> + depends on OF
> + depends on DRM_MIPI_DSI
> + depends on BACKLIGHT_CLASS_DEVICE
> + help
> + Say Y here if you want to support for AUO KD101N80_45NA WUXGA PANEL
> + DSI Video Mode panel
> +
> endmenu
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 78e3dc376bdd..1056933bdf2e 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -28,3 +28,4 @@ obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7701) +=
> panel-sitronix-st7701.o
> obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
> obj-$(CONFIG_DRM_PANEL_TPO_TPG110) += panel-tpo-tpg110.o
> obj-$(CONFIG_DRM_PANEL_TRULY_NT35597_WQXGA) += panel-truly-nt35597.o
> +obj-$(CONFIG_DRM_PANEL_AUO_KD101N80_45NA) += panel-auo-kd101n80-45na.o
> diff --git a/drivers/gpu/drm/panel/panel-auo-kd101n80-45na.c
> b/drivers/gpu/drm/panel/panel-auo-kd101n80-45na.c
> new file mode 100644
> index ..ab7bfc059e8a
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-auo-kd101n80-45na.c
> @@ -0,0 +1,352 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 MediaTek Inc.
> + * Author: Jitao Shi
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +struct auo_panel {
> + struct drm_panel base;
> + struct mipi_dsi_device *dsi;
> +
> + struct backlight_device *backlight;
> + struct regulator *pp1800;
> + struct regulator *avee;
> + struct regulator *avdd;
> + struct gpio_desc *enable_gpio;
> +
> + bool prepared;
> + bool enabled;
> +
> + const struct drm_display_mode *mode;
> +};
> +
> +static inline struct auo_panel *to_auo_panel(struct drm_panel *panel)
> +{
> + return container_of(panel, struct auo_panel, base);
> +}
> +
> +static int auo_panel_init(struct auo_panel *auo)
> +{
> + struct drm_panel *panel = >base;
> + int err;
> +
> + err = mipi_dsi_dcs_exit_sleep_mode(auo->dsi);
> + if (err < 0) {
> + DRM_DEV_ERROR(panel->dev, "failed to exit sleep mode: %d\n",
> + err);
> + return err;
> + }
> +
> + /* T3.1*/
> + msleep(120);
> +
> + err = mipi_dsi_dcs_set_display_on(auo->dsi);
> + if (err < 0) {
> + DRM_DEV_ERROR(panel->dev, "failed to set display on: %d\n",
> + err);
> + }
> + /* T3.1 + T3.2: > 200ms */
> + msleep(120);
> +
> + return err;
> +}
> +
> +static int auo_panel_off(struct auo_panel *auo)
> +{
> + struct mipi_dsi_device *dsi = auo->dsi;
> + int ret;
> +
> + dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> +
> + ret = mipi_dsi_dcs_set_display_off(dsi);
> + if (ret < 0)
> + return ret;
> +
> + ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int auo_panel_disable(struct drm_panel *panel)
> +{
> + struct auo_panel *auo = to_auo_panel(panel);
> +
> + if (!auo->enabled)
> + return 0;
> +
> + backlight_disable(auo->backlight);
> +
> + auo->enabled = false;
> +
> + return 0;
> +}
> +
> +static int auo_panel_unprepare(struct drm_panel *panel)
> +{
> + struct auo_panel *auo = to_auo_panel(panel);
> + int ret;
> +
> + if (!auo->prepared)
> + return 0;
> +
> + ret = auo_panel_off(auo);
> + if (ret < 0) {
> + dev_err(panel->dev, "failed to set panel off: %d\n", ret);
> + return ret;
> +
Re: [2/2] drm/panel: support for BOE tv101wum-n16 wuxga dsi video mode panel
Hi Jitao.
> +
> +enum dsi_cmd_type {
> + INIT_GENENIC_CMD,
> + INIT_DCS_CMD,
> + DELAY_CMD,
> +};
> +
> +struct panel_init_cmd {
> + enum dsi_cmd_type type;
> + size_t len;
> + const char *data;
> +};
> +
> +#define _INIT_CMD(...) { \
> + .type = INIT_GENENIC_CMD,\
> + .len = sizeof((char[]){__VA_ARGS__}), \
> + .data = (char[]){__VA_ARGS__} }
This macro is not used - so all code around INIT_GENENIC_CMD can be
deleted.
Sam
[PATCH] dma-fence: Signal all callbacks from dma_fence_release()
This is an illegal scenario, to free the fence whilst there are pending
callbacks. Currently, we emit a WARN and then cast aside the callbacks
leaving them dangling. Alternatively, we could set an error on the fence
and then signal fence so that any dependency chains from the fence can
be tidied up, and if they care they can check for the error.
Signed-off-by: Chris Wilson
Reviewed-by: Gustavo Padovan
---
drivers/dma-buf/dma-fence.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 227a19476d56..59ac96ec7ba8 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -248,8 +248,25 @@ void dma_fence_release(struct kref *kref)
trace_dma_fence_destroy(fence);
- /* Failed to signal before release, could be a refcounting issue */
- WARN_ON(!list_empty(>cb_list));
+ if (WARN(!list_empty(>cb_list),
+"Fence %s:%s:%llx:%llx released with pending signals!\n",
+fence->ops->get_driver_name(fence),
+fence->ops->get_timeline_name(fence),
+fence->context, fence->seqno)) {
+ unsigned long flags;
+
+ /*
+* Failed to signal before release, likely a refcounting issue.
+*
+* This should never happen, but if it does make sure that we
+* don't leave chains dangling. We set the error flag first
+* so that the callbacks know this signal is due to an error.
+*/
+ spin_lock_irqsave(fence->lock, flags);
+ fence->error = -EDEADLK;
+ dma_fence_signal_locked(fence);
+ spin_unlock_irqrestore(fence->lock, flags);
+ }
if (fence->ops->release)
fence->ops->release(fence);
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [1/2] dt-bindings: display: panel: add AUO kd101n80-45na panel bindings
Hi Jitao.
Binding doc for this panel looks good.
With added description for avdd + avee it is:
Reviewed-by: Sam Ravnborg
On Sat, Jun 08, 2019 at 07:23:41PM +0800, Jitao Shi wrote:
> Add documentation for auo kd101n80-45na panel.
>
> Signed-off-by: Jitao Shi
> ---
> .../display/panel/auo,kd101n80-45na.txt | 34 +++
> 1 file changed, 34 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/panel/auo,kd101n80-45na.txt
>
> diff --git
> a/Documentation/devicetree/bindings/display/panel/auo,kd101n80-45na.txt
> b/Documentation/devicetree/bindings/display/panel/auo,kd101n80-45na.txt
> new file mode 100644
> index ..7715cf703431
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/auo,kd101n80-45na.txt
> @@ -0,0 +1,34 @@
> +AUO Corporation 10.1" WUXGA TFT LCD panel
> +
> +Required properties:
> +- compatible: should be "auo,kd101n80-45na"
> +- reg: the virtual channel number of a DSI peripheral
> +- enable-gpios: a GPIO spec for the enable pin
> +- pp1800-supply: core voltage supply
> +- avdd-supply:
> +- avee-supply:
Descriptions are missing for the 2 x supply.
> +- backlight: phandle of the backlight device attached to the panel
> +
> +The device node can contain one 'port' child node with one child
> +'endpoint' node, according to the bindings defined in
> +media/video-interfaces.txt. This node should describe panel's video bus.
> +
> +Example:
> + {
> + ...
> + panel@0 {
> + compatible = "auo,kd101n80-45na";
> + reg = <0>;
> + enable-gpios = < 45 0>;
> + avdd-supply = <_lcd>;
> + avee-supply = <_lcd>;
> + pp1800-supply = <_lcd>;
> + backlight = <_lcd0>;
> + status = "okay";
> + port {
> + panel_in: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> + };
> +};
> --
> 2.21.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [2/2] drm/panel: support for BOE tv101wum-n16 wuxga dsi video mode panel
Hi Jitao.
Thanks for this new nice driver - a few comments follows.
Sam
On Sat, Jun 08, 2019 at 03:02:30PM +0800, Jitao Shi wrote:
> Add driver for BOE tv101wum-nl6 panel is a 10.1" 1200x1920 panel.
Like in the binding there is some confusion between nl6 versus n16.
Here the subject says one thing, and the changelogn another thing.
>
> Signed-off-by: Jitao Shi
> ---
> drivers/gpu/drm/panel/Kconfig | 10 +
> drivers/gpu/drm/panel/Makefile| 1 +
> .../gpu/drm/panel/panel-boe-tv101wum-nl6.c| 700 ++
> 3 files changed, 711 insertions(+)
> create mode 100644 drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index e36dbb4df867..5dad028e35f0 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -272,4 +272,14 @@ config DRM_PANEL_TRULY_NT35597_WQXGA
> help
> Say Y here if you want to enable support for Truly NT35597 WQXGA Dual
> DSI
> Video Mode panel
> +
> +config DRM_PANEL_BOE_TV101WUM_NL6
> + tristate "BOE TV101WUM 1200x1920 panel"
> + depends on OF
> + depends on DRM_MIPI_DSI
> + depends on BACKLIGHT_CLASS_DEVICE
> + help
> + Say Y here if you want to support for BOE TV101WUM WUXGA PANEL
> + DSI Video Mode panel
> +
New panel CONFIG symbols are added in alphabetically order.
> endmenu
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 78e3dc376bdd..fc5944231aa8 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -28,3 +28,4 @@ obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7701) +=
> panel-sitronix-st7701.o
> obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
> obj-$(CONFIG_DRM_PANEL_TPO_TPG110) += panel-tpo-tpg110.o
> obj-$(CONFIG_DRM_PANEL_TRULY_NT35597_WQXGA) += panel-truly-nt35597.o
> +obj-$(CONFIG_DRM_PANEL_BOE_TV101WUM_NL6) += panel-boe-tv101wum-nl6.o
Drivers are added in alphabetically order after their CONFIG symbol.
> diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
> b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
> new file mode 100644
> index ..19590664c337
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
> @@ -0,0 +1,700 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 MediaTek Inc.
> + * Author: Jitao Shi
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
drmP.h is deprecated - do not use in new drivers.
> +#include
> +#include
> +#include
> +
> +#include
> +
> +struct boe_panel {
> + struct drm_panel base;
> + struct mipi_dsi_device *dsi;
> +
> + struct backlight_device *backlight;
> + struct regulator *pp1800;
> + struct regulator *avee;
> + struct regulator *avdd;
> + struct gpio_desc *enable_gpio;
> +
> + bool prepared;
> + bool enabled;
> +
> + const struct drm_display_mode *mode;
> +};
> +
> +enum dsi_cmd_type {
> + INIT_GENENIC_CMD,
> + INIT_DCS_CMD,
> + DELAY_CMD,
> +};
> +
> +struct panel_init_cmd {
> + enum dsi_cmd_type type;
> + size_t len;
> + const char *data;
> +};
> +
> +#define _INIT_CMD(...) { \
> + .type = INIT_GENENIC_CMD,\
> + .len = sizeof((char[]){__VA_ARGS__}), \
> + .data = (char[]){__VA_ARGS__} }
> +
> +#define _INIT_DCS_CMD(...) { \
> + .type = INIT_DCS_CMD, \
> + .len = sizeof((char[]){__VA_ARGS__}), \
> + .data = (char[]){__VA_ARGS__} }
> +
> +#define _INIT_DELAY_CMD(...) { \
> + .type = DELAY_CMD,\
> + .len = sizeof((char[]){__VA_ARGS__}), \
> + .data = (char[]){__VA_ARGS__} }
> +
> +static const struct panel_init_cmd boe_init_cmd[] = {
In the following there are some empty lines.
If they have any semantic purpose please add a comment.
If not, then drop the empty lines.
> + _INIT_DELAY_CMD(24),
> + _INIT_DCS_CMD(0xB0, 0x05),
> + _INIT_DCS_CMD(0xB1, 0xE5),
> + _INIT_DCS_CMD(0xB3, 0x52),
> +
> + _INIT_DCS_CMD(0xB0, 0x00),
> + _INIT_DCS_CMD(0xB3, 0x88),
> + _INIT_DCS_CMD(0xB0, 0x04),
> + _INIT_DCS_CMD(0xB8, 0x00),
> +
> + _INIT_DCS_CMD(0xB0, 0x00),
> + _INIT_DCS_CMD(0xB6, 0x03),
> + _INIT_DCS_CMD(0xBA, 0x8B),
> + _INIT_DCS_CMD(0xBF, 0x1A),
> + _INIT_DCS_CMD(0xC0, 0x0F),
> + _INIT_DCS_CMD(0xC2, 0x0C),
> + _INIT_DCS_CMD(0xC3, 0x02),
> + _INIT_DCS_CMD(0xC4, 0x0C),
> + _INIT_DCS_CMD(0xC5, 0x02),
> +
> + _INIT_DCS_CMD(0xB0, 0x01),
> + _INIT_DCS_CMD(0xE0, 0x26),
> + _INIT_DCS_CMD(0xE1, 0x26),
> + _INIT_DCS_CMD(0xDC, 0x00),
> + _INIT_DCS_CMD(0xDD, 0x00),
> + _INIT_DCS_CMD(0xCC, 0x26),
> + _INIT_DCS_CMD(0xCD, 0x26),
> + _INIT_DCS_CMD(0xC8, 0x00),
> + _INIT_DCS_CMD(0xC9, 0x00),
> + _INIT_DCS_CMD(0xD2, 0x03),
> + _INIT_DCS_CMD(0xD3, 0x03),
> + _INIT_DCS_CMD(0xE6, 0x04),
> + _INIT_DCS_CMD(0xE7, 0x04),
> +
Re: [1/2] dt-bindngs: display: panel: Add BOE tv101wum-nl6 panel bindings
Hi Jitao
Always good to see more panels added.
On Sat, Jun 08, 2019 at 03:02:29PM +0800, Jitao Shi wrote:
> Add documentation for boe tv101wum-n16 panel.
The changelog says tv101wum-n16 but the compatible says tv101wum-nl6.
Please pick the right one and be consistent. (number one '1' versus
character 'l')
>
> Signed-off-by: Jitao Shi
> ---
> .../display/panel/boe,tv101wum-nl6.txt| 34 +++
> 1 file changed, 34 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.txt
>
> diff --git
> a/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.txt
> b/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.txt
> new file mode 100644
> index ..2a84735d742d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.txt
> @@ -0,0 +1,34 @@
> +Boe Corporation 10.1" WUXGA TFT LCD panel
> +
> +Required properties:
> +- compatible: should be "boe,tv101wum"
The example uses "boe,tv101wum-nl6", which matches the filename.
Please fix so it is the same all over.
> +- reg: the virtual channel number of a DSI peripheral
> +- enable-gpios: a GPIO spec for the enable pin
> +- pp1800-supply: core voltage supply
> +- avdd-supply:
> +- avee-supply:
Missing descriptions - please add.
> +- backlight: phandle of the backlight device attached to the panel
> +
> +The device node can contain one 'port' child node with one child
> +'endpoint' node, according to the bindings defined in
> +media/video-interfaces.txt. This node should describe panel's video bus.
> +
> +Example:
> + {
> + ...
> + panel@0 {
> + compatible = "boe,tv101wum-nl6";
> + reg = <0>;
> + enable-gpios = < 45 0>;
> + avdd-supply = <_lcd>;
> + avee-supply = <_lcd>;
> + pp1800-supply = <_lcd>;
> + backlight = <_lcd0>;
> + status = "okay";
> + port {
> + panel_in: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> + };
> +};
> \ No newline at end of file
Please add newline.
Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] dma-fence: Signal all callbacks from dma_fence_release()
This is an illegal scenario, to free the fence whilst there are pending
callbacks. Currently, we emit a WARN and then cast aside the callbacks
leaving them dangling. Alternatively, we could set an error on the fence
and then signal fence so that any dependency chains from the fence can
be tidied up, and if they care they can check for the error.
Signed-off-by: Chris Wilson
Reviewed-by: Gustavo Padovan
---
drivers/dma-buf/dma-fence.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 227a19476d56..59ac96ec7ba8 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -248,8 +248,25 @@ void dma_fence_release(struct kref *kref)
trace_dma_fence_destroy(fence);
- /* Failed to signal before release, could be a refcounting issue */
- WARN_ON(!list_empty(>cb_list));
+ if (WARN(!list_empty(>cb_list),
+"Fence %s:%s:%llx:%llx released with pending signals!\n",
+fence->ops->get_driver_name(fence),
+fence->ops->get_timeline_name(fence),
+fence->context, fence->seqno)) {
+ unsigned long flags;
+
+ /*
+* Failed to signal before release, likely a refcounting issue.
+*
+* This should never happen, but if it does make sure that we
+* don't leave chains dangling. We set the error flag first
+* so that the callbacks know this signal is due to an error.
+*/
+ spin_lock_irqsave(fence->lock, flags);
+ fence->error = -EDEADLK;
+ dma_fence_signal_locked(fence);
+ spin_unlock_irqrestore(fence->lock, flags);
+ }
if (fence->ops->release)
fence->ops->release(fence);
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 10/33] docs: fb: convert docs to ReST and rename to *.rst
Hi Mauro,
On Sun, Jun 9, 2019 at 4:29 AM Mauro Carvalho Chehab
wrote:
> The conversion is actually:
> - add blank lines and identation in order to identify paragraphs;
> - fix tables markups;
> - add some lists markups;
> - mark literal blocks;
> - adjust title markups.
>
> At its new index.rst, let's add a :orphan: while this is not linked to
> the main index.rst file, in order to avoid build warnings.
>
> Signed-off-by: Mauro Carvalho Chehab
Thanks!
> --- a/Documentation/fb/framebuffer.txt
> +++ b/Documentation/fb/framebuffer.rst
> @@ -1,5 +1,6 @@
> - The Frame Buffer Device
> - ---
> +===
> +The Frame Buffer Device
> +===
>
> Maintained by Geert Uytterhoeven
I'm happy to see this line dropped ;-)
> Last revised: May 10, 2001
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
