Re: [PATCH v1 1/2] drm/bridge/synopsys: dsi: add power on/off optional phy ops
On 27.05.2019 12:21, Yannick Fertré wrote:
> Add power on & off optional physical operation functions, helpful to
> program specific registers of the DSI physical part.
>
> Signed-off-by: Yannick Fertré
Reviewed-by: Andrzej Hajda
--
Regards
Andrzej
> ---
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 8
> include/drm/bridge/dw_mipi_dsi.h | 2 ++
> 2 files changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index e915ae8..5bb676f 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -775,6 +775,10 @@ static void dw_mipi_dsi_clear_err(struct dw_mipi_dsi
> *dsi)
> static void dw_mipi_dsi_bridge_post_disable(struct drm_bridge *bridge)
> {
> struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge);
> + const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops;
> +
> + if (phy_ops->power_off)
> + phy_ops->power_off(dsi->plat_data->priv_data);
>
> /*
>* Switch to command mode before panel-bridge post_disable &
> @@ -874,11 +878,15 @@ static void dw_mipi_dsi_bridge_mode_set(struct
> drm_bridge *bridge,
> static void dw_mipi_dsi_bridge_enable(struct drm_bridge *bridge)
> {
> struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge);
> + const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops;
>
> /* Switch to video mode for panel-bridge enable & panel enable */
> dw_mipi_dsi_set_mode(dsi, MIPI_DSI_MODE_VIDEO);
> if (dsi->slave)
> dw_mipi_dsi_set_mode(dsi->slave, MIPI_DSI_MODE_VIDEO);
> +
> + if (phy_ops->power_on)
> + phy_ops->power_on(dsi->plat_data->priv_data);
> }
>
> static enum drm_mode_status
> diff --git a/include/drm/bridge/dw_mipi_dsi.h
> b/include/drm/bridge/dw_mipi_dsi.h
> index 7d3dd69..df6eda6 100644
> --- a/include/drm/bridge/dw_mipi_dsi.h
> +++ b/include/drm/bridge/dw_mipi_dsi.h
> @@ -14,6 +14,8 @@ struct dw_mipi_dsi;
>
> struct dw_mipi_dsi_phy_ops {
> int (*init)(void *priv_data);
> + void (*power_on)(void *priv_data);
> + void (*power_off)(void *priv_data);
> int (*get_lane_mbps)(void *priv_data,
>const struct drm_display_mode *mode,
>unsigned long mode_flags, u32 lanes, u32 format,
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110898] [Patch] to compile amdgpu-dkms 9.10 on debian stretch & buster
https://bugs.freedesktop.org/show_bug.cgi?id=110898 Bug ID: 110898 Summary: [Patch] to compile amdgpu-dkms 9.10 on debian stretch & buster Product: DRI Version: DRI git Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: fabstz...@yahoo.fr Created attachment 144517 --> https://bugs.freedesktop.org/attachment.cgi?id=144517&action=edit Patch to compile amdgpu-dkms 9.10 on debian buster (debian 10) and stretch Hello, Please find attached a patch so that one can compile the amdgpu-dkms 9.10 driver toward kernel 4.19 on debian stretch (with stretch-backports), and also on debian buster (version 10) which runs linux-4.19 The patch is made of: - a change to the Makefile so that it can compile well on debian and find the kernel version (using [make kernelversion]) - a few changes to the code by renaming some functions whose names changes to allow compilation. For information, you may find my current procedure to compile the driver on my debian setup here https://wiki.debian.org/How%20to%20install%20official%20AMDGPU%20linux%20driver%20with%20kernel%204.19.x%20on%20Stretch%20and%20Buster Kind regards -- 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 v1 2/2] drm/stm: dsi: add power on/off phy ops
On 27.05.2019 12:21, Yannick Fertré wrote:
> These new physical operations are helpful to power_on/off the dsi
> wrapper. If the dsi wrapper is powered in video mode, the display
> controller (ltdc) register access will hang when DSI fifos are full.
>
> Signed-off-by: Yannick Fertré
Reviewed-by: Andrzej Hajda
--
Regards
Andrzej
> ---
> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 21 -
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> index 01db020..0ab32fe 100644
> --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> @@ -210,10 +210,27 @@ static int dw_mipi_dsi_phy_init(void *priv_data)
> if (ret)
> DRM_DEBUG_DRIVER("!TIMEOUT! waiting PLL, let's continue\n");
>
> + return 0;
> +}
> +
> +static void dw_mipi_dsi_phy_power_on(void *priv_data)
> +{
> + struct dw_mipi_dsi_stm *dsi = priv_data;
> +
> + DRM_DEBUG_DRIVER("\n");
> +
> /* Enable the DSI wrapper */
> dsi_set(dsi, DSI_WCR, WCR_DSIEN);
> +}
>
> - return 0;
> +static void dw_mipi_dsi_phy_power_off(void *priv_data)
> +{
> + struct dw_mipi_dsi_stm *dsi = priv_data;
> +
> + DRM_DEBUG_DRIVER("\n");
> +
> + /* Disable the DSI wrapper */
> + dsi_clear(dsi, DSI_WCR, WCR_DSIEN);
> }
>
> static int
> @@ -287,6 +304,8 @@ dw_mipi_dsi_get_lane_mbps(void *priv_data, const struct
> drm_display_mode *mode,
>
> static const struct dw_mipi_dsi_phy_ops dw_mipi_dsi_stm_phy_ops = {
> .init = dw_mipi_dsi_phy_init,
> + .power_on = dw_mipi_dsi_phy_power_on,
> + .power_off = dw_mipi_dsi_phy_power_off,
> .get_lane_mbps = dw_mipi_dsi_get_lane_mbps,
> };
>
[Bug 110898] [Patch] to compile amdgpu-dkms 9.10 on debian stretch & buster
https://bugs.freedesktop.org/show_bug.cgi?id=110898 Fab Stz changed: What|Removed |Added Component|DRM/AMDgpu |DRM/AMDgpu-pro -- 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
[PATCH v3 0/2] drm/komeda: Add SMMU support on Komeda driver
Hi, This serie aims at adding the support for SMMU on Komeda driver. Also updates the device-tree doc about how to enable SMMU by devicetree. This patch series depends on: - https://patchwork.freedesktop.org/series/58710/ - https://patchwork.freedesktop.org/series/59000/ - https://patchwork.freedesktop.org/series/59002/ - https://patchwork.freedesktop.org/series/61650/ Changes since v1: - Rebase to the patch in which convert dp_wait_cond() was changed to return -ETIMEDOUT and update d71_disconnect_iommu() to be consistent. - If connected IOMMU failed, set mdev->iommu as NULL. Changes since v2: - Correct the code flow by not returning -ETIMEDOUT if dp_wait_cond() returns zero in d71_connect_iommu(). Thanks, Lowry Lowry Li (Arm Technology China) (2): drm/komeda: Adds SMMU support dt/bindings: drm/komeda: Adds SMMU support for D71 devicetree .../devicetree/bindings/display/arm,komeda.txt | 7 .../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 + 7 files changed, 90 insertions(+) -- 1.9.1
[PATCH v3 2/2] dt/bindings: drm/komeda: Adds SMMU support for D71 devicetree
From: "Lowry Li (Arm Technology China)"
Updates the device-tree doc about how to enable SMMU by devicetree.
Signed-off-by: Lowry Li (Arm Technology China)
Reviewed-by: Liviu Dudau
Reviewed-by: James Qian Wang (Arm Technology China)
---
Documentation/devicetree/bindings/display/arm,komeda.txt | 7 +++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/arm,komeda.txt
b/Documentation/devicetree/bindings/display/arm,komeda.txt
index 02b2265..b12c045 100644
--- a/Documentation/devicetree/bindings/display/arm,komeda.txt
+++ b/Documentation/devicetree/bindings/display/arm,komeda.txt
@@ -11,6 +11,10 @@ Required properties:
- "pclk": for the APB interface clock
- #address-cells: Must be 1
- #size-cells: Must be 0
+- iommus: configure the stream id to IOMMU, Must be configured if want to
+enable iommu in display. for how to configure this node please reference
+devicetree/bindings/iommu/arm,smmu-v3.txt,
+devicetree/bindings/iommu/iommu.txt
Required properties for sub-node: pipeline@nq
Each device contains one or two pipeline sub-nodes (at least one), each
@@ -44,6 +48,9 @@ Example:
interrupts = <0 168 4>;
clocks = <&dpu_mclk>, <&dpu_aclk>;
clock-names = "mclk", "pclk";
+ iommus = <&smmu 0>, <&smmu 1>, <&smmu 2>, <&smmu 3>,
+ <&smmu 4>, <&smmu 5>, <&smmu 6>, <&smmu 7>,
+ <&smmu 8>, <&smmu 9>;
dp0_pipe0: pipeline@0 {
clocks = <&fpgaosc2>, <&dpu_aclk>;
--
1.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/2] drm/komeda: Adds SMMU support
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(&state->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..f88d93a 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) {
+ 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
index c92e161..e80e673 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
@@ -253,6 +253,19 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
dev->dma_parms = &mdev->dma_parms;
dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
+ mdev->iommu = iommu_get_domain_for_dev(mdev->dev);
+ if (!mdev->iommu)
+ DRM_INFO("continue without IOMMU support!\n");
+
+ if (mdev->iommu && mdev->funcs->connect_iommu) {
+ err = mdev->funcs->connect_iommu(mdev);
+ if (err) {
+ mdev->
Re: [PATCH v1 0/2] dw-mipi-dsi: add power on & off optional phy ops and update stm
On 27.05.2019 12:21, Yannick Fertré wrote: > These patches fix a bug concerning an access issue to display controler (ltdc) > registers. > If the physical layer of the DSI is started too early then the fifo DSI are > full > very quickly which implies ltdc register's access hang up. To avoid this > problem, it is necessary to start the DSI physical layer only when the bridge > is enable. > > Yannick Fertré (2): > drm/bridge/synopsys: dsi: add power on/off optional phy ops > drm/stm: dsi: add power on/off phy ops > > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 8 > drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 21 - > include/drm/bridge/dw_mipi_dsi.h | 2 ++ > 3 files changed, 30 insertions(+), 1 deletion(-) > > -- > 2.7.4 > > > Queued both patches to drm-misc-next. -- Regards Andrzej ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/2] drm/komeda: Add SMMU support on Komeda driver
Hi, This serie aims at adding the support for SMMU on Komeda driver. Also updates the device-tree doc about how to enable SMMU by devicetree. This patch series depends on: - https://patchwork.freedesktop.org/series/58710/ - https://patchwork.freedesktop.org/series/59000/ - https://patchwork.freedesktop.org/series/59002/ - https://patchwork.freedesktop.org/series/61650/ Changes since v1: - Rebase to the patch in which convert dp_wait_cond() was changed to return -ETIMEDOUT and update d71_disconnect_iommu() to be consistent. - If connected IOMMU failed, set mdev->iommu as NULL. Changes since v2: - Correct the code flow by not returning -ETIMEDOUT if dp_wait_cond() returns zero in d71_connect_iommu(). Thanks, Lowry Lowry Li (Arm Technology China) (2): drm/komeda: Adds SMMU support dt/bindings: drm/komeda: Adds SMMU support for D71 devicetree .../devicetree/bindings/display/arm,komeda.txt | 7 .../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 + 7 files changed, 90 insertions(+) -- 1.9.1
[PATCH v3 1/2] drm/komeda: Adds SMMU support
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(&state->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..3dab5077 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) {
+ DRM_ERROR("connect to TCU timeout!\n");
+ malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
+ return ret;
+ }
+
+ 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
index c92e161..e80e673 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
@@ -253,6 +253,19 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
dev->dma_parms = &mdev->dma_parms;
dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
+ mdev->iommu = iommu_get_domain_for_dev(mdev->dev);
+ if (!mdev->iommu)
+ DRM_INFO("continue without IOMMU support!\n");
+
+ if (mdev->iommu && mdev->funcs->connect_iommu) {
+ err = mdev->funcs->connect_iommu(mdev);
+ if (err) {
+ mdev->iommu
[PATCH v3 2/2] dt/bindings: drm/komeda: Adds SMMU support for D71 devicetree
From: "Lowry Li (Arm Technology China)"
Updates the device-tree doc about how to enable SMMU by devicetree.
Signed-off-by: Lowry Li (Arm Technology China)
Reviewed-by: Liviu Dudau
Reviewed-by: James Qian Wang (Arm Technology China)
---
Documentation/devicetree/bindings/display/arm,komeda.txt | 7 +++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/arm,komeda.txt
b/Documentation/devicetree/bindings/display/arm,komeda.txt
index 02b2265..b12c045 100644
--- a/Documentation/devicetree/bindings/display/arm,komeda.txt
+++ b/Documentation/devicetree/bindings/display/arm,komeda.txt
@@ -11,6 +11,10 @@ Required properties:
- "pclk": for the APB interface clock
- #address-cells: Must be 1
- #size-cells: Must be 0
+- iommus: configure the stream id to IOMMU, Must be configured if want to
+enable iommu in display. for how to configure this node please reference
+devicetree/bindings/iommu/arm,smmu-v3.txt,
+devicetree/bindings/iommu/iommu.txt
Required properties for sub-node: pipeline@nq
Each device contains one or two pipeline sub-nodes (at least one), each
@@ -44,6 +48,9 @@ Example:
interrupts = <0 168 4>;
clocks = <&dpu_mclk>, <&dpu_aclk>;
clock-names = "mclk", "pclk";
+ iommus = <&smmu 0>, <&smmu 1>, <&smmu 2>, <&smmu 3>,
+ <&smmu 4>, <&smmu 5>, <&smmu 6>, <&smmu 7>,
+ <&smmu 8>, <&smmu 9>;
dp0_pipe0: pipeline@0 {
clocks = <&fpgaosc2>, <&dpu_aclk>;
--
1.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 6/8] drm/mgag200: Rewrite cursor handling
Hi Am 11.06.19 um 17:33 schrieb Daniel Vetter: > On Tue, Jun 11, 2019 at 2:32 PM Thomas Zimmermann wrote: >> >> Hi >> >> Am 05.06.19 um 11:58 schrieb Gerd Hoffmann: >>> On Tue, Jun 04, 2019 at 05:41:59PM +0200, Thomas Zimmermann wrote: The cursor handling in mgag200 is complicated to understand. It touches a number of different BOs, but doesn't really use all of them. Rewriting the cursor update reduces the amount of cursor state. There are two BOs for double-buffered HW updates. The source BO updates the one that is currently not displayed and then switches buffers. Explicit BO locking has been removed from the code. BOs are simply pinned and unpinned in video RAM. >>> >>> Cursors are not that big after all, so maybe pin the two BOs for >>> double-buffering permanently in vram to simplify things further? >>> >>> Also factoring out the code which updates the two BOs to a separate >>> function should help making the code more readable. >> >> The cursor handling in the ast driver is similar, but uses one single BO >> to hold both cursor buffers. I'm thinking about how to turn both >> implementations into a generic helper for legacy cursors (i.e., low >> number of colors or palette). This would also be helpful for my work on >> fbdev porting. >> >> One idea is to adapt deferred I/O. DRM would expose an ARGB shadow >> buffer to userspace and let the mmap implementation update the HW >> buffers (including dithering, palette setup, etc.). > > No mmap games needed with kms, we expect userspace to give us an ioctl > call in all cases. fbdev is the legacy horrors that works differently. Thanks for clarifying this. Conversion should be much easier this way. I saw the dirty callback and the DIRTYFB ioctl, but I don't saw anything in Weston that calls it. So I assumed that it's obsolete or optional. Best regards Thomas > So for cursors, assuming you have universal cursors, you just need to > update the shadowed copy in the prepare_fb plane hook. For > non-universal plane drivers you need to do that somewhere in your > set/move_cursor hooks (I think both of them). Aside: For non-cursors > there's also the dirtyfb ioctl, which serves the same purpose. > > Cheers, Daniel > >> >> Best regards >> Thomas >> >>> But even as-is the patch is a step into the right direction. >>> >>> Acked-by: Gerd Hoffmann >>> >>> cheers, >>> Gerd >>> >> >> -- >> Thomas Zimmermann >> Graphics Driver Developer >> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany >> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah >> HRB 21284 (AG Nürnberg) >> > > -- Thomas Zimmermann Graphics Driver Developer SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 21284 (AG Nürnberg) signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 5/9] drm/ast: Pin framebuffer BO during dirty update
Hi
Am 11.06.19 um 22:29 schrieb Sam Ravnborg:
> Hi Thomas.
>
> On Tue, Jun 11, 2019 at 03:03:40PM +0200, Thomas Zimmermann wrote:
>> Another explicit lock operation of a GEM VRAM BO is located in AST's
>> framebuffer update code. Instead of locking the BO, we pin it to wherever
>> it is.
>>
>> v2:
>> * update with pin flag of 0
>>
>> Signed-off-by: Thomas Zimmermann
>> ---
>> drivers/gpu/drm/ast/ast_fb.c | 33 -
>> 1 file changed, 16 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
>> index 05f45222b702..b404b51c2c55 100644
>> --- a/drivers/gpu/drm/ast/ast_fb.c
>> +++ b/drivers/gpu/drm/ast/ast_fb.c
>> @@ -48,32 +48,31 @@ static void ast_dirty_update(struct ast_fbdev *afbdev,
>> int x, int y, int width, int height)
>> {
>> int i;
>> -struct drm_gem_object *obj;
>> struct drm_gem_vram_object *gbo;
>> int src_offset, dst_offset;
>> int bpp = afbdev->afb.base.format->cpp[0];
>> -int ret = -EBUSY;
>> +int ret;
>> u8 *dst;
>> bool unmap = false;
>> bool store_for_later = false;
>> int x2, y2;
>> unsigned long flags;
>>
>> -obj = afbdev->afb.obj;
>> -gbo = drm_gem_vram_of_gem(obj);
>> -
>> -/* Try to lock the BO. If we fail with -EBUSY then
>> - * the BO is being moved and we should store up the
>> - * damage until later.
>> - */
>> -if (drm_can_sleep())
>> -ret = drm_gem_vram_lock(gbo, true);
>> -if (ret) {
>> -if (ret != -EBUSY)
>> -return;
>> -
>> +gbo = drm_gem_vram_of_gem(afbdev->afb.obj);
>> +
>> +if (drm_can_sleep()) {
>
> While touching this code, anyway we could get rid of drm_can_sleep()?
> I only ask because Daniel V. said that we should not use it.
> This was some months ago during some ehader file clean-up, so nothing
> in particular related to the ast driver.
I'm aware of what is commented in the header and the todo file. Do you
have a link to this discussion?
In any case, I'd prefer to fix this in a separate patch set. I have
patches that replace the ast and mgag200 fbdev consoles with generic
code. That might be the right place.
Best regards
Thomas
>
> Sam
>
>> +/* We pin the BO so it won't be moved during the
>> + * update. The actual location, video RAM or system
>> + * memory, is not important.
>> + */
>> +ret = drm_gem_vram_pin(gbo, 0);
>> +if (ret) {
>> +if (ret != -EBUSY)
>> +return;
>> +store_for_later = true;
>> +}
>> +} else
>> store_for_later = true;
>> -}
>>
>> x2 = x + width - 1;
>> y2 = y + height - 1;
>> @@ -126,7 +125,7 @@ static void ast_dirty_update(struct ast_fbdev *afbdev,
>> drm_gem_vram_kunmap(gbo);
>>
>> out:
>> -drm_gem_vram_unlock(gbo);
>> +drm_gem_vram_unpin(gbo);
>> }
>>
>> static void ast_fillrect(struct fb_info *info,
>> --
>> 2.21.0
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/7] drm/bridge: split some definitions of ANX78xx to dedicated headers
On 04.06.2019 14:22, Torsten Duwe wrote: > From: Icenowy Zheng > > Some definitions currently in analogix-anx78xx.h are not restricted to > the ANX78xx series, but also applicable to other DisplayPort > transmitters by Analogix. > > Split out them to dedicated headers, and make analogix-anx78xx.h include > them. > > Signed-off-by: Icenowy Zheng > Signed-off-by: Vasily Khoruzhick > Signed-off-by: Torsten Duwe > --- > drivers/gpu/drm/bridge/analogix/analogix-anx78xx.h | 465 > + > .../gpu/drm/bridge/analogix/analogix-i2c-dptx.h| 256 > .../drm/bridge/analogix/analogix-i2c-txcommon.h| 242 +++ > 3 files changed, 503 insertions(+), 460 deletions(-) > create mode 100644 drivers/gpu/drm/bridge/analogix/analogix-i2c-dptx.h > create mode 100644 drivers/gpu/drm/bridge/analogix/analogix-i2c-txcommon.h > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.h > b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.h > index 38753c870137..3fbe2c3244fd 100644 > --- a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.h > +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.h > @@ -11,13 +11,15 @@ > * GNU General Public License for more details. > * > */ > - > #ifndef __ANX78xx_H > #define __ANX78xx_H > > -#define TX_P00x70 > +#include "analogix-i2c-dptx.h" > +#include "analogix-i2c-txcommon.h" > + > +#define TX_P0ANALOGIX_I2C_DPTX > #define TX_P10x7a > -#define TX_P20x72 > +#define TX_P2ANALOGIX_I2C_TXCOMMON > > #define RX_P00x7e > #define RX_P10x80 > @@ -225,463 +227,6 @@ > #define SP_CLEAR_AVMUTE BIT(4) > #define SP_SET_AVMUTEBIT(0) > > -/***/ > -/* Register definition of device address 0x70 */ > -/***/ > - > -/* HDCP Status Register */ > -#define SP_TX_HDCP_STATUS_REG0x00 > -#define SP_AUTH_FAIL BIT(5) > -#define SP_AUTHEN_PASS BIT(1) > - > -/* HDCP Control Register 0 */ > -#define SP_HDCP_CTRL0_REG0x01 > -#define SP_RX_REPEATER BIT(6) > -#define SP_RE_AUTH BIT(5) > -#define SP_SW_AUTH_OKBIT(4) > -#define SP_HARD_AUTH_EN BIT(3) > -#define SP_HDCP_ENC_EN BIT(2) > -#define SP_BKSV_SRM_PASS BIT(1) > -#define SP_KSVLIST_VLD BIT(0) > -/* HDCP Function Enabled */ > -#define SP_HDCP_FUNCTION_ENABLED (BIT(0) | BIT(1) | BIT(2) | BIT(3)) > - > -/* HDCP Receiver BSTATUS Register 0 */ > -#define SP_HDCP_RX_BSTATUS0_REG 0x1b > -/* HDCP Receiver BSTATUS Register 1 */ > -#define SP_HDCP_RX_BSTATUS1_REG 0x1c > - > -/* HDCP Embedded "Blue Screen" Content Registers */ > -#define SP_HDCP_VID0_BLUE_SCREEN_REG 0x2c > -#define SP_HDCP_VID1_BLUE_SCREEN_REG 0x2d > -#define SP_HDCP_VID2_BLUE_SCREEN_REG 0x2e > - > -/* HDCP Wait R0 Timing Register */ > -#define SP_HDCP_WAIT_R0_TIME_REG 0x40 > - > -/* HDCP Link Integrity Check Timer Register */ > -#define SP_HDCP_LINK_CHECK_TIMER_REG 0x41 > - > -/* HDCP Repeater Ready Wait Timer Register */ > -#define SP_HDCP_RPTR_RDY_WAIT_TIME_REG 0x42 > - > -/* HDCP Auto Timer Register */ > -#define SP_HDCP_AUTO_TIMER_REG 0x51 > - > -/* HDCP Key Status Register */ > -#define SP_HDCP_KEY_STATUS_REG 0x5e > - > -/* HDCP Key Command Register */ > -#define SP_HDCP_KEY_COMMAND_REG 0x5f > -#define SP_DISABLE_SYNC_HDCP BIT(2) > - > -/* OTP Memory Key Protection Registers */ > -#define SP_OTP_KEY_PROTECT1_REG 0x60 > -#define SP_OTP_KEY_PROTECT2_REG 0x61 > -#define SP_OTP_KEY_PROTECT3_REG 0x62 > -#define SP_OTP_PSW1 0xa2 > -#define SP_OTP_PSW2 0x7e > -#define SP_OTP_PSW3 0xc6 > - > -/* DP System Control Registers */ > -#define SP_DP_SYSTEM_CTRL_BASE (0x80 - 1) > -/* Bits for DP System Control Register 2 */ > -#define SP_CHA_STA BIT(2) > -/* Bits for DP System Control Register 3 */ > -#define SP_HPD_STATUSBIT(6) > -#define SP_STRM_VALIDBIT(2) > -/* Bits for DP System Control Register 4 */ > -#define SP_ENHANCED_MODE BIT(3) > - > -/* DP Video Control Register */ > -#define SP_DP_VIDEO_CTRL_REG 0x84 > -#define SP_COLOR_F_MASK 0x06 > -#define SP_COLOR_F_SHIFT 1 > -#define SP_BPC_MASK 0xe0 > -#define SP_BPC_SHIFT 5 > -# define SP_BPC_6BITS 0x00 > -# define SP_BPC_8BITS
Re: [PATCH 05/20] clk: renesas: r8a7795: Add CMM clocks
On Thu, Jun 6, 2019 at 4:22 PM Jacopo Mondi wrote:
> Add clock definitions for CMM units on Renesas R-Car Gen3 H3.
>
> Signed-off-by: Jacopo Mondi
Reviewed-by: Geert Uytterhoeven
i.e. will queue in clk-renesas-for-v5.3.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v2 3/7] drm/bridge: extract some Analogix I2C DP common code
On 04.06.2019 14:22, Torsten Duwe wrote:
> From: Icenowy Zheng
>
> Some code can be shared within different DP bridges by Analogix.
> Extract them to analogix_dp.
>
> Signed-off-by: Icenowy Zheng
> Signed-off-by: Vasily Khoruzhick
> Signed-off-by: Torsten Duwe
> ---
> drivers/gpu/drm/bridge/analogix/Makefile | 2 +-
> drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 146 +
> .../gpu/drm/bridge/analogix/analogix-i2c-dptx.c| 173
> +
> .../gpu/drm/bridge/analogix/analogix-i2c-dptx.h| 3 +
> 4 files changed, 178 insertions(+), 146 deletions(-)
> create mode 100644 drivers/gpu/drm/bridge/analogix/analogix-i2c-dptx.c
>
> diff --git a/drivers/gpu/drm/bridge/analogix/Makefile
> b/drivers/gpu/drm/bridge/analogix/Makefile
> index 6fcbfd3ee560..7623b9b80167 100644
> --- a/drivers/gpu/drm/bridge/analogix/Makefile
> +++ b/drivers/gpu/drm/bridge/analogix/Makefile
> @@ -1,4 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0-only
> -analogix_dp-objs := analogix_dp_core.o analogix_dp_reg.o
> +analogix_dp-objs := analogix_dp_core.o analogix_dp_reg.o analogix-i2c-dptx.o
> obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
> obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp.o
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> index c09aaf93ae1b..f36ae51c641d 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> @@ -45,8 +45,6 @@
> #define I2C_IDX_RX_P14
>
> #define XTAL_CLK 270 /* 27M */
> -#define AUX_CH_BUFFER_SIZE 16
> -#define AUX_WAIT_TIMEOUT_MS 15
>
> static const u8 anx78xx_i2c_addresses[] = {
> [I2C_IDX_TX_P0] = TX_P0,
> @@ -109,153 +107,11 @@ static int anx78xx_clear_bits(struct regmap *map, u8
> reg, u8 mask)
> return regmap_update_bits(map, reg, mask, 0);
> }
>
> -static bool anx78xx_aux_op_finished(struct anx78xx *anx78xx)
> -{
> - unsigned int value;
> - int err;
> -
> - err = regmap_read(anx78xx->map[I2C_IDX_TX_P0], SP_DP_AUX_CH_CTRL2_REG,
> - &value);
> - if (err < 0)
> - return false;
> -
> - return (value & SP_AUX_EN) == 0;
> -}
> -
> -static int anx78xx_aux_wait(struct anx78xx *anx78xx)
> -{
> - unsigned long timeout;
> - unsigned int status;
> - int err;
> -
> - timeout = jiffies + msecs_to_jiffies(AUX_WAIT_TIMEOUT_MS) + 1;
> -
> - while (!anx78xx_aux_op_finished(anx78xx)) {
> - if (time_after(jiffies, timeout)) {
> - if (!anx78xx_aux_op_finished(anx78xx)) {
> - DRM_ERROR("Timed out waiting AUX to finish\n");
> - return -ETIMEDOUT;
> - }
> -
> - break;
> - }
> -
> - usleep_range(1000, 2000);
> - }
> -
> - /* Read the AUX channel access status */
> - err = regmap_read(anx78xx->map[I2C_IDX_TX_P0], SP_AUX_CH_STATUS_REG,
> - &status);
> - if (err < 0) {
> - DRM_ERROR("Failed to read from AUX channel: %d\n", err);
> - return err;
> - }
> -
> - if (status & SP_AUX_STATUS) {
> - DRM_ERROR("Failed to wait for AUX channel (status: %02x)\n",
> - status);
> - return -ETIMEDOUT;
> - }
> -
> - return 0;
> -}
> -
> -static int anx78xx_aux_address(struct anx78xx *anx78xx, unsigned int addr)
> -{
> - int err;
> -
> - err = regmap_write(anx78xx->map[I2C_IDX_TX_P0], SP_AUX_ADDR_7_0_REG,
> -addr & 0xff);
> - if (err)
> - return err;
> -
> - err = regmap_write(anx78xx->map[I2C_IDX_TX_P0], SP_AUX_ADDR_15_8_REG,
> -(addr & 0xff00) >> 8);
> - if (err)
> - return err;
> -
> - /*
> - * DP AUX CH Address Register #2, only update bits[3:0]
> - * [7:4] RESERVED
> - * [3:0] AUX_ADDR[19:16], Register control AUX CH address.
> - */
> - err = regmap_update_bits(anx78xx->map[I2C_IDX_TX_P0],
> - SP_AUX_ADDR_19_16_REG,
> - SP_AUX_ADDR_19_16_MASK,
> - (addr & 0xf) >> 16);
> -
> - if (err)
> - return err;
> -
> - return 0;
> -}
> -
> static ssize_t anx78xx_aux_transfer(struct drm_dp_aux *aux,
> struct drm_dp_aux_msg *msg)
> {
> struct anx78xx *anx78xx = container_of(aux, struct anx78xx, aux);
> - u8 ctrl1 = msg->request;
> - u8 ctrl2 = SP_AUX_EN;
> - u8 *buffer = msg->buffer;
> - int err;
> -
> - /* The DP AUX transmit and receive buffer has 16 bytes. */
> - if (WARN_ON(msg->size > AUX_CH_BUFFER_SIZE))
> - return -E2BIG;
> -
> - /* Zero-sized messages specify address-only transactions. */
> - if (msg->size < 1)
> -
Re: [PATCH 06/20] clk: renesas: r8a77965: Add CMM clocks
On Thu, Jun 6, 2019 at 4:22 PM Jacopo Mondi wrote:
> Add clock definitions for CMM units on Renesas R-Car Gen3 M3-N.
>
> Signed-off-by: Jacopo Mondi
Reviewed-by: Geert Uytterhoeven
i.e. will queue in clk-renesas-for-v5.3.
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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
On 6/11/2019 4:24 PM, Viresh Kumar wrote: On 20-03-19, 15:19, Rajendra Nayak wrote: From: Stephen Boyd Doing this allows us to call this API with any rate requested and have it not need to match in the OPP table. Instead, we'll round the rate up to the nearest OPP that we see so that we can get the voltage or level that's required for that OPP. This supports users of OPP that want to specify the 'fmax' tables of a device instead of every single frequency that they need. And for devices that required the exact frequency, we can rely on the clk framework to round the rate to the nearest supported frequency instead of the OPP framework to do so. Note that this may affect drivers that don't want the clk framework to do rounding, but instead want the OPP table to do the rounding for them. Do we have that case? Should we add some flag to the OPP table to indicate this and then not have that flag set when there isn't an OPP table for the device and also introduce a property like 'opp-use-clk' to tell the table that it should use the clk APIs to round rates instead of OPP? Signed-off-by: Stephen Boyd Signed-off-by: Rajendra Nayak --- []... I see a logical problem with this patch. Suppose the clock driver supports following frequencies: 500M, 800M, 1G, 1.2G and the OPP table contains following list: 500M, 1G, 1.2G (i.e. missing 800M). Now 800M should never get programmed as it isn't part of the OPP table. But if you pass 600M to opp-set-rate, then it will end up selecting 800M as clock driver will round up to the closest value. correct Even if no one is doing this right now, it is a sensible usecase, specially during testing of patches and I don't think we should avoid it. What exactly is the use case for which we need this patch ? Like the changelog says 'This supports users of OPP that want to specify the 'fmax' tables of a device instead of every single frequency that they need' so the 'fmax' tables basically say what the max frequency the device can operate at for a given performance state/voltage level. so in your example it would be for instance 500M, Perf state = 2 1G, Perf state = 3 1.2G, Perf state = 4 Now when the device wants to operate at say 800Mhz, you need to set the Perf state to 3, so this patch basically avoids you having to put those additional OPPs in the table which would otherwise look something like this 500M, Perf state = 2 800M, Perf state = 3 <-- redundant OPP 1G, Perf state = 3 1.2G, Perf state = 4 Your example had just 1 missing entry in the 'fmax' tables in reality its a lot more, atleast on all qualcomm platforms. -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/7] drm/bridge: Prepare Analogix anx6345 support
On 04.06.2019 14:22, Torsten Duwe wrote:
> Add bit definitions required for the anx6345 and add a
> sanity check in anx_dp_aux_transfer.
>
> Signed-off-by: Icenowy Zheng
> Signed-off-by: Vasily Khoruzhick
> Signed-off-by: Torsten Duwe
Reviewed-by: Andrzej Hajda
--
Regards
Andrzej
> ---
> drivers/gpu/drm/bridge/analogix/analogix-i2c-dptx.c | 2 +-
> drivers/gpu/drm/bridge/analogix/analogix-i2c-dptx.h | 8
> drivers/gpu/drm/bridge/analogix/analogix-i2c-txcommon.h | 3 +++
> 3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-i2c-dptx.c
> b/drivers/gpu/drm/bridge/analogix/analogix-i2c-dptx.c
> index d6016f789d80..e9d2ed4d410d 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix-i2c-dptx.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix-i2c-dptx.c
> @@ -124,7 +124,7 @@ ssize_t anx_dp_aux_transfer(struct regmap *map_dptx,
> else/* For non-zero-sized set the length field. */
> ctrl1 |= (msg->size - 1) << SP_AUX_LENGTH_SHIFT;
>
> - if ((msg->request & DP_AUX_I2C_READ) == 0) {
> + if ((msg->size > 0) && ((msg->request & DP_AUX_I2C_READ) == 0)) {
> /* When WRITE | MOT write values to data buffer */
> err = regmap_bulk_write(map_dptx,
> SP_DP_BUF_DATA0_REG, buffer,
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-i2c-dptx.h
> b/drivers/gpu/drm/bridge/analogix/analogix-i2c-dptx.h
> index 30436c88f181..95ab89eecc60 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix-i2c-dptx.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix-i2c-dptx.h
> @@ -83,7 +83,11 @@
> #define SP_CHA_STA BIT(2)
> /* Bits for DP System Control Register 3 */
> #define SP_HPD_STATUSBIT(6)
> +#define SP_HPD_FORCE BIT(5)
> +#define SP_HPD_CTRL BIT(4)
> #define SP_STRM_VALIDBIT(2)
> +#define SP_STRM_FORCEBIT(1)
> +#define SP_STRM_CTRL BIT(0)
> /* Bits for DP System Control Register 4 */
> #define SP_ENHANCED_MODE BIT(3)
>
> @@ -128,6 +132,9 @@
> #define SP_LINK_BW_SET_MASK 0x1f
> #define SP_INITIAL_SLIM_M_AUD_SELBIT(5)
>
> +/* DP Lane Count Setting Register */
> +#define SP_DP_LANE_COUNT_SET_REG 0xa1
> +
> /* DP Training Pattern Set Register */
> #define SP_DP_TRAINING_PATTERN_SET_REG 0xa2
>
> @@ -141,6 +148,7 @@
>
> /* DP Link Training Control Register */
> #define SP_DP_LT_CTRL_REG0xa8
> +#define SP_DP_LT_INPROGRESS 0x80
> #define SP_LT_ERROR_TYPE_MASK0x70
> # define SP_LT_NO_ERROR 0x00
> # define SP_LT_AUX_WRITE_ERROR 0x01
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-i2c-txcommon.h
> b/drivers/gpu/drm/bridge/analogix/analogix-i2c-txcommon.h
> index f48293f86f9d..e3391a50b5d1 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix-i2c-txcommon.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix-i2c-txcommon.h
> @@ -188,6 +188,9 @@
> #define SP_VBIT BIT(1)
> #define SP_AUDIO_LAYOUT BIT(0)
>
> +/* Analog Debug Register 1 */
> +#define SP_ANALOG_DEBUG1_REG 0xdc
> +
> /* Analog Debug Register 2 */
> #define SP_ANALOG_DEBUG2_REG 0xdd
> #define SP_FORCE_SW_OFF_BYPASS 0x20
Re: [PATCH 07/20] clk: renesas: r8a77990: Add CMM clocks
On Thu, Jun 6, 2019 at 4:25 PM Jacopo Mondi wrote:
> Add clock definitions for CMM units on Renesas R-Car Gen3 E3.
>
> Signed-off-by: Jacopo Mondi
Reviewed-by: Geert Uytterhoeven
i.e. will queue in clk-renesas-for-v5.3.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 08/20] clk: renesas: r8a77995: Add CMM clocks
On Thu, Jun 6, 2019 at 4:22 PM Jacopo Mondi wrote:
> Add clock definitions for CMM units on Renesas R-Car Gen3 D3.
>
> Signed-off-by: Jacopo Mondi
Reviewed-by: Geert Uytterhoeven
i.e. will queue in clk-renesas-for-v5.3.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH] dma-buf: refcount the attachment for cache_sgt_mapping
Am 12.06.19 um 03:22 schrieb Nicolin Chen:
> Commit f13e143e7444 ("dma-buf: start caching of sg_table objects v2")
> added a support of caching the sgt pointer into an attach pointer to
> let users reuse the sgt pointer without another mapping. However, it
> might not totally work as most of dma-buf callers are doing attach()
> and map_attachment() back-to-back, using drm_prime.c for example:
> drm_gem_prime_import_dev() {
> attach = dma_buf_attach() {
> /* Allocating a new attach */
> attach = kzalloc();
> /* */
> return attach;
> }
> dma_buf_map_attachment(attach, direction) {
> /* attach->sgt would be always empty as attach is new */
> if (attach->sgt) {
> /* Reuse attach->sgt */
> }
> /* Otherwise, map it */
> attach->sgt = map();
> }
> }
>
> So, for a cache_sgt_mapping use case, it would need to get the same
> attachment pointer in order to reuse its sgt pointer. So this patch
> adds a refcount to the attach() function and lets it search for the
> existing attach pointer by matching the dev pointer.
I don't think that this is a good idea.
We use sgt caching as workaround for locking order problems and want to
remove it again in the long term.
So what is the actual use case of this?
Regards,
Christian.
>
> Signed-off-by: Nicolin Chen
> ---
> drivers/dma-buf/dma-buf.c | 23 +++
> include/linux/dma-buf.h | 2 ++
> 2 files changed, 25 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index f4104a21b069..d0260553a31c 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -559,6 +559,21 @@ struct dma_buf_attachment *dma_buf_attach(struct dma_buf
> *dmabuf,
> if (WARN_ON(!dmabuf || !dev))
> return ERR_PTR(-EINVAL);
>
> + /* cache_sgt_mapping requires to reuse the same attachment pointer */
> + if (dmabuf->ops->cache_sgt_mapping) {
> + mutex_lock(&dmabuf->lock);
> +
> + /* Search for existing attachment and increase its refcount */
> + list_for_each_entry(attach, &dmabuf->attachments, node) {
> + if (dev != attach->dev)
> + continue;
> + atomic_inc_not_zero(&attach->refcount);
> + goto unlock_attach;
> + }
> +
> + mutex_unlock(&dmabuf->lock);
> + }
> +
> attach = kzalloc(sizeof(*attach), GFP_KERNEL);
> if (!attach)
> return ERR_PTR(-ENOMEM);
> @@ -575,6 +590,9 @@ struct dma_buf_attachment *dma_buf_attach(struct dma_buf
> *dmabuf,
> }
> list_add(&attach->node, &dmabuf->attachments);
>
> + atomic_set(&attach->refcount, 1);
> +
> +unlock_attach:
> mutex_unlock(&dmabuf->lock);
>
> return attach;
> @@ -599,6 +617,11 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct
> dma_buf_attachment *attach)
> if (WARN_ON(!dmabuf || !attach))
> return;
>
> + /* Decrease the refcount for cache_sgt_mapping use cases */
> + if (dmabuf->ops->cache_sgt_mapping &&
> + atomic_dec_return(&attach->refcount))
> + return;
> +
> if (attach->sgt)
> dmabuf->ops->unmap_dma_buf(attach, attach->sgt, attach->dir);
>
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index 8a327566d7f4..65f12212ca2e 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -333,6 +333,7 @@ struct dma_buf {
>* @dev: device attached to the buffer.
>* @node: list of dma_buf_attachment.
>* @sgt: cached mapping.
> + * @refcount: refcount of the attachment for the same device.
>* @dir: direction of cached mapping.
>* @priv: exporter specific attachment data.
>*
> @@ -350,6 +351,7 @@ struct dma_buf_attachment {
> struct device *dev;
> struct list_head node;
> struct sg_table *sgt;
> + atomic_t refcount;
> enum dma_data_direction dir;
> void *priv;
> };
Re: [PATCH v2 5/9] drm/ast: Pin framebuffer BO during dirty update
Hi Thomas. > > > > While touching this code, anyway we could get rid of drm_can_sleep()? > > I only ask because Daniel V. said that we should not use it. > > This was some months ago during some ehader file clean-up, so nothing > > in particular related to the ast driver. > > I'm aware of what is commented in the header and the todo file. Do you > have a link to this discussion? I managed to dig up this link: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1887389.html But that does not provide any additional technical details. > In any case, I'd prefer to fix this in a separate patch set. I have > patches that replace the ast and mgag200 fbdev consoles with generic > code. That might be the right place. Using generic code, and thus deleting the old code seems like a good plan. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[GIT,PULL] mediatek drm fixes for 5.2
Hi Dave, Daniel: This include unbind error fix, clock control flow refinement, and PRIME mmap with page offset. Regards, CK The following changes since commit a188339ca5a396acc588e5851ed7e19f66b0ebd9: Linux 5.2-rc1 (2019-05-19 15:47:09 -0700) are available in the Git repository at: https://github.com/ckhu-mediatek/linux.git-tags.git mediatek-drm-fixes-5.2 for you to fetch changes up to 2458d9d6d94be982b917e93c61a89b4426f32e31: drm/mediatek: call mtk_dsi_stop() after mtk_drm_crtc_atomic_disable() (2019-06-04 09:54:42 +0800) Hsin-Yi Wang (5): drm/mediatek: fix unbind functions drm/mediatek: unbind components in mtk_drm_unbind() drm/mediatek: call drm_atomic_helper_shutdown() when unbinding driver drm/mediatek: clear num_pipes when unbind driver drm/mediatek: call mtk_dsi_stop() after mtk_drm_crtc_atomic_disable() Yongqiang Niu (2): drm/mediatek: adjust ddp clock control flow drm/mediatek: respect page offset for PRIME mmap calls drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 30 ++ drivers/gpu/drm/mediatek/mtk_drm_drv.c | 8 +++- drivers/gpu/drm/mediatek/mtk_drm_gem.c | 7 ++- drivers/gpu/drm/mediatek/mtk_dsi.c | 12 +++- 4 files changed, 26 insertions(+), 31 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dma-buf: refcount the attachment for cache_sgt_mapping
Hi Christian,
Thanks for the quick reply.
On Wed, Jun 12, 2019 at 07:45:38AM +, Koenig, Christian wrote:
> Am 12.06.19 um 03:22 schrieb Nicolin Chen:
> > Commit f13e143e7444 ("dma-buf: start caching of sg_table objects v2")
> > added a support of caching the sgt pointer into an attach pointer to
> > let users reuse the sgt pointer without another mapping. However, it
> > might not totally work as most of dma-buf callers are doing attach()
> > and map_attachment() back-to-back, using drm_prime.c for example:
> > drm_gem_prime_import_dev() {
> > attach = dma_buf_attach() {
> > /* Allocating a new attach */
> > attach = kzalloc();
> > /* */
> > return attach;
> > }
> > dma_buf_map_attachment(attach, direction) {
> > /* attach->sgt would be always empty as attach is new */
> > if (attach->sgt) {
> > /* Reuse attach->sgt */
> > }
> > /* Otherwise, map it */
> > attach->sgt = map();
> > }
> > }
> >
> > So, for a cache_sgt_mapping use case, it would need to get the same
> > attachment pointer in order to reuse its sgt pointer. So this patch
> > adds a refcount to the attach() function and lets it search for the
> > existing attach pointer by matching the dev pointer.
>
> I don't think that this is a good idea.
>
> We use sgt caching as workaround for locking order problems and want to
> remove it again in the long term.
Oh. I thought it was for a performance improving purpose. It may
be a misunderstanding then.
> So what is the actual use case of this?
We have some similar downstream changes at dma_buf to reduce the
overhead from multiple clients of the same device doing attach()
and map_attachment() calls for the same dma_buf.
We haven't used DRM/GRM_PRIME yet but I am also curious would it
benefit DRM also if we reduce this overhead in the dma_buf?
Thanks
Nicolin
Re: [PATCH] dma-buf: refcount the attachment for cache_sgt_mapping
Am 12.06.19 um 10:02 schrieb Nicolin Chen:
> Hi Christian,
>
> Thanks for the quick reply.
>
> On Wed, Jun 12, 2019 at 07:45:38AM +, Koenig, Christian wrote:
>> Am 12.06.19 um 03:22 schrieb Nicolin Chen:
>>> Commit f13e143e7444 ("dma-buf: start caching of sg_table objects v2")
>>> added a support of caching the sgt pointer into an attach pointer to
>>> let users reuse the sgt pointer without another mapping. However, it
>>> might not totally work as most of dma-buf callers are doing attach()
>>> and map_attachment() back-to-back, using drm_prime.c for example:
>>> drm_gem_prime_import_dev() {
>>> attach = dma_buf_attach() {
>>> /* Allocating a new attach */
>>> attach = kzalloc();
>>> /* */
>>> return attach;
>>> }
>>> dma_buf_map_attachment(attach, direction) {
>>> /* attach->sgt would be always empty as attach is new */
>>> if (attach->sgt) {
>>> /* Reuse attach->sgt */
>>> }
>>> /* Otherwise, map it */
>>> attach->sgt = map();
>>> }
>>> }
>>>
>>> So, for a cache_sgt_mapping use case, it would need to get the same
>>> attachment pointer in order to reuse its sgt pointer. So this patch
>>> adds a refcount to the attach() function and lets it search for the
>>> existing attach pointer by matching the dev pointer.
>> I don't think that this is a good idea.
>>
>> We use sgt caching as workaround for locking order problems and want to
>> remove it again in the long term.
> Oh. I thought it was for a performance improving purpose. It may
> be a misunderstanding then.
>
>> So what is the actual use case of this?
> We have some similar downstream changes at dma_buf to reduce the
> overhead from multiple clients of the same device doing attach()
> and map_attachment() calls for the same dma_buf.
I don't think that this is a good idea over all. A driver calling attach
for the same buffer is doing something wrong in the first place and we
should not work around this in the DMA-buf handling.
> We haven't used DRM/GRM_PRIME yet but I am also curious would it
> benefit DRM also if we reduce this overhead in the dma_buf?
No, not at all.
Regards,
Christian.
>
> Thanks
> Nicolin
Re: [PATCH v3 3/6] drm/modes: Allow to specify rotation and reflection on the commandline
On Tue, 11 Jun 2019, Maxime Ripard wrote:
> Hi Noralf,
>
> On Fri, Apr 19, 2019 at 10:53:28AM +0200, Noralf Trønnes wrote:
>> Den 18.04.2019 18.40, skrev Noralf Trønnes:
>> >
>> >
>> > Den 18.04.2019 14.41, skrev Maxime Ripard:
>> >> Rotations and reflections setup are needed in some scenarios to initialise
>> >> properly the initial framebuffer. Some drivers already had a bunch of
>> >> quirks to deal with this, such as either a private kernel command line
>> >> parameter (omapdss) or on the device tree (various panels).
>> >>
>> >> In order to accomodate this, let's create a video mode parameter to deal
>> >> with the rotation and reflexion.
>> >>
>> >> Signed-off-by: Maxime Ripard
>> >> ---
>> >> drivers/gpu/drm/drm_client_modeset.c | 10 +++-
>> >> drivers/gpu/drm/drm_modes.c | 110 ++--
>> >> include/drm/drm_connector.h | 9 ++-
>> >> 3 files changed, 109 insertions(+), 20 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/drm_client_modeset.c
>> >> b/drivers/gpu/drm/drm_client_modeset.c
>> >> index f2869c82510c..15145d2c86d5 100644
>> >> --- a/drivers/gpu/drm/drm_client_modeset.c
>> >> +++ b/drivers/gpu/drm/drm_client_modeset.c
>> >> @@ -823,6 +823,7 @@ EXPORT_SYMBOL(drm_client_modeset_probe);
>> >> bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned
>> >> int *rotation)
>> >> {
>> >> struct drm_connector *connector = modeset->connectors[0];
>> >> + struct drm_cmdline_mode *cmdline;
>> >> struct drm_plane *plane = modeset->crtc->primary;
>> >> u64 valid_mask = 0;
>> >> unsigned int i;
>> >> @@ -844,6 +845,15 @@ bool drm_client_panel_rotation(struct drm_mode_set
>> >> *modeset, unsigned int *rotat
>> >> *rotation = DRM_MODE_ROTATE_0;
>> >> }
>> >>
>> >> + /**
>> >> + * We want the rotation on the command line to overwrite
>> >> + * whatever comes from the panel.
>> >> + */
>> >> + cmdline = &connector->cmdline_mode;
>> >> + if (cmdline->specified &&
>> >> + cmdline->rotation != DRM_MODE_ROTATE_0)
>> >
>> > I believe you need to drop that second check, otherwise rotate=0 will
>> > not overwrite panel rotation.
>> >
>> >> + *rotation = cmdline->rotation;
>>
>> I remembered that you wanted this to propagate to DRM userspace. That's
>> not happening here.
>
> It's propated to the userspace through the plane's rotation property,
> I just checked.
>
>> The only way I see for that to happen, is to set
>> ->panel_orientation. And to repeat myself, imo that makes
>> 'orientation' a better name for this video= option.
>
> orientation and rotation are two different things to me. The
> orientation of a panel for example is absolute, while the rotation is
> a transformation. In this particular case, I think that both the
> orientation and the rotation should be taken into account, with the
> orientation being the default state, and the hardware / panel will
> tell us that, while the rotation would be a transformation from that
> default to whatever the user wants.
>
> More importantly, the orientation is a property of the hardware (ie,
> how the display has been assembled), while the rotation is a software
> construct.
FWIW, agreed. The immutable orientation property is exposed using the
drm_connector_init_panel_orientation_property() call. You then rotate to
take the orientation into account, to not display stuff sideways or
upside down wrt the natural orientation of the device.
BR,
Jani.
>
> And if the property being used to expose that is the rotation, I guess
> it would make sense to just use the same name and remain consistent.
>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [GIT PULL] omapdrm changes for 5.3
On 11/06/2019 20:36, Daniel Vetter wrote: omapdrm changes for 5.3 - Add support for DSI command mode displays Thanks, pulled. Finally :) Hm why? Pull is less than a day old, and I didn't see an older one that Dave or me missed ... That was directed at me =). I've been reluctant to merge the DSI command mode support, as it's based on the omapdrm specific driver model, instead of drm bridges/panels. Especially as Laurent has been creating huge patch serieses converting omapdrm to the drm bridge/panel model, I've been worried about conflicts and making Laurent's work more difficult. So... The DSI command mode series has been floating around for a long time. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110795] Unable to install on latest Ubuntu (19.04)
https://bugs.freedesktop.org/show_bug.cgi?id=110795 Christian König changed: What|Removed |Added Resolution|--- |INVALID Status|NEW |RESOLVED --- Comment #14 from Christian König --- (In reply to Rolf from comment #12) > Perhaps I'm trying to do something unnecessary under Ubuntu? Yes, exactly that seems to be the case here. Either the drivers coming with Ubuntu should be sufficient or you should use a PPA to get bleeding edge drivers. As Alex already noted as well the -pro drivers are for workstation use cases where the user has a specific version of a distribution. Marking this bug as invalid for now. -- 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 1/9] drm/gem-vram: Support pinning buffers to current location
On Tue, Jun 11, 2019 at 03:03:36PM +0200, Thomas Zimmermann wrote:
> Pinning a buffer prevents it from being moved to a different memory
> location. For some operations, such as buffer updates, it is not
> important where the buffer is located. Setting the pin function's
> pl_flag argument to 0 will pin the buffer to whereever it is stored.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/drm_gem_vram_helper.c | 12
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
> b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 42ad80888df7..214f54b8920b 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -224,7 +224,9 @@ EXPORT_SYMBOL(drm_gem_vram_offset);
> *
> * Pinning a buffer object ensures that it is not evicted from
> * a memory region. A pinned buffer object has to be unpinned before
> - * it can be pinned to another region.
> + * it can be pinned to another region. If the pl_flag argument is 0,
> + * the buffer is pinned at its current location (video RAM or system
> + * memory).
> *
> * Returns:
> * 0 on success, or
> @@ -242,7 +244,9 @@ int drm_gem_vram_pin(struct drm_gem_vram_object *gbo,
> unsigned long pl_flag)
> if (gbo->pin_count)
> goto out;
>
> - drm_gem_vram_placement(gbo, pl_flag);
> + if (pl_flag)
> + drm_gem_vram_placement(gbo, pl_flag);
> +
> for (i = 0; i < gbo->placement.num_placement; ++i)
> gbo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT;
>
> @@ -691,7 +695,7 @@ int drm_gem_vram_driver_gem_prime_pin(struct
> drm_gem_object *gem)
> {
> struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem);
>
> - return drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
> + return drm_gem_vram_pin(gbo, 0);
Not sure this is a good idea here. If the bo happens to be in sysram
it can't be displayed any more.
> - ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
> + ret = drm_gem_vram_pin(gbo, 0);
Likewise.
cheers,
Gerd
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/9] drm/ast: Pin and map cursor source BO during update
On Tue, Jun 11, 2019 at 03:03:39PM +0200, Thomas Zimmermann wrote:
> The ast driver used to lock the cursor source BO during updates. Locking
> should be done internally by the BO's implementation, so we pin it instead
> to system memory. The mapping information is also stored in the BO. No
> need to have an extra argument to the kmap function.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/ast/ast_mode.c | 29 ++---
> 1 file changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index cb6e8249a7db..be3d91d7fde5 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -1183,7 +1183,6 @@ static int ast_cursor_set(struct drm_crtc *crtc,
> u64 gpu_addr;
> u32 csum;
> int ret;
> - struct ttm_bo_kmap_obj uobj_map;
> u8 *src, *dst;
> bool src_isiomem, dst_isiomem;
> if (!handle) {
> @@ -1201,15 +1200,13 @@ static int ast_cursor_set(struct drm_crtc *crtc,
> }
> gbo = drm_gem_vram_of_gem(obj);
>
> - ret = drm_gem_vram_lock(gbo, false);
> + ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_SYSTEM);
For a temporary pin like this one using pl_flag = 0 should be fine.
cheers,
Gerd
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dma-buf: refcount the attachment for cache_sgt_mapping
Hi Christian,
On Wed, Jun 12, 2019 at 08:05:53AM +, Koenig, Christian wrote:
> Am 12.06.19 um 10:02 schrieb Nicolin Chen:
> > Hi Christian,
> >
> > Thanks for the quick reply.
> >
> > On Wed, Jun 12, 2019 at 07:45:38AM +, Koenig, Christian wrote:
> >> Am 12.06.19 um 03:22 schrieb Nicolin Chen:
> >>> Commit f13e143e7444 ("dma-buf: start caching of sg_table objects v2")
> >>> added a support of caching the sgt pointer into an attach pointer to
> >>> let users reuse the sgt pointer without another mapping. However, it
> >>> might not totally work as most of dma-buf callers are doing attach()
> >>> and map_attachment() back-to-back, using drm_prime.c for example:
> >>> drm_gem_prime_import_dev() {
> >>> attach = dma_buf_attach() {
> >>> /* Allocating a new attach */
> >>> attach = kzalloc();
> >>> /* */
> >>> return attach;
> >>> }
> >>> dma_buf_map_attachment(attach, direction) {
> >>> /* attach->sgt would be always empty as attach is new */
> >>> if (attach->sgt) {
> >>> /* Reuse attach->sgt */
> >>> }
> >>> /* Otherwise, map it */
> >>> attach->sgt = map();
> >>> }
> >>> }
> >>>
> >>> So, for a cache_sgt_mapping use case, it would need to get the same
> >>> attachment pointer in order to reuse its sgt pointer. So this patch
> >>> adds a refcount to the attach() function and lets it search for the
> >>> existing attach pointer by matching the dev pointer.
> >> I don't think that this is a good idea.
> >>
> >> We use sgt caching as workaround for locking order problems and want to
> >> remove it again in the long term.
> > Oh. I thought it was for a performance improving purpose. It may
> > be a misunderstanding then.
> >
> >> So what is the actual use case of this?
> > We have some similar downstream changes at dma_buf to reduce the
> > overhead from multiple clients of the same device doing attach()
> > and map_attachment() calls for the same dma_buf.
>
> I don't think that this is a good idea over all. A driver calling attach
> for the same buffer is doing something wrong in the first place and we
> should not work around this in the DMA-buf handling.
>
> > We haven't used DRM/GRM_PRIME yet but I am also curious would it
> > benefit DRM also if we reduce this overhead in the dma_buf?
>
> No, not at all.
>From you replies, in a summary, does it means that there won't be a case
of DRM having a dma_buf attaching to the same device, i.e. multiple calls
of drm_gem_prime_import() function with same parameters of dev + dma_buf?
If so, we can just ignore/drop this patch. Sorry for the misunderstanding.
Thanks
Nicolin
[Bug 110888] 5.0.21 kernel crash when many GPU app run concurrently , error msg: amdgpu_vm_validate_pt_bos() failed. , Not enough memory for command submission!
https://bugs.freedesktop.org/show_bug.cgi?id=110888 --- Comment #2 from Christian König --- Looks like a NULL pointer check is missing somewhere in amdgpu_vm_init() to me. But in general you are running out of system memory, not video memory. So whatever you try to do here won't work in general unless you either add more system memory or add a swap file. -- 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 6/7] dt-bindings: Add ANX6345 DP/eDP transmitter binding
On 04.06.2019 14:23, Torsten Duwe wrote:
> The anx6345 is an ultra-low power DisplayPort/eDP transmitter designed
> for portable devices.
>
> Add a binding document for it.
>
> Signed-off-by: Icenowy Zheng
> Signed-off-by: Vasily Khoruzhick
> Reviewed-by: Rob Herring
> Signed-off-by: Torsten Duwe
> Reviewed-by: Laurent Pinchart
> ---
> .../devicetree/bindings/display/bridge/anx6345.txt | 57
> ++
> 1 file changed, 57 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/bridge/anx6345.txt
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/anx6345.txt
> b/Documentation/devicetree/bindings/display/bridge/anx6345.txt
> new file mode 100644
> index ..bd63f6ac107e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/anx6345.txt
> @@ -0,0 +1,57 @@
> +Analogix ANX6345 eDP Transmitter
> +
> +
> +The ANX6345 is an ultra-low power Full-HD eDP transmitter designed for
> +portable devices.
> +
> +Required properties:
> +
> + - compatible: "analogix,anx6345"
> + - reg : I2C address of the device
> + - reset-gpios : Which GPIO to use for reset
You have not specified it's active state, since in driver's code you
named it RESETN I guess it should be active low.
> + - dvdd12-supply : Regulator for 1.2V digital core power.
> + - dvdd25-supply : Regulator for 2.5V digital core power.
> + - Video port for LVTTL input, using the DT bindings defined in [1].
Please assign port number for input (I guess 0).
> +
> +Optional properties:
> +
> + - Video port for eDP output (panel or connector) using the DT bindings
> + defined in [1].
Shouldn't it be also required?
Regards
Andrzej
> +
> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +Example:
> +
> +anx6345: anx6345@38 {
> + compatible = "analogix,anx6345";
> + reg = <0x38>;
> + reset-gpios = <&pio 3 24 GPIO_ACTIVE_LOW>; /* PD24 */
> + dvdd25-supply = <®_dldo2>;
> + dvdd12-supply = <®_fldo1>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + anx6345_in: port@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;
> + anx6345_in_tcon0: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&tcon0_out_anx6345>;
> + };
> + };
> +
> + anx6345_out: port@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> +
> + anx6345_out_panel: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&panel_in_edp>;
> + };
> + };
> + };
> +};
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 7/9] drm/mgag200: Rewrite cursor handling
On Tue, Jun 11, 2019 at 03:03:42PM +0200, Thomas Zimmermann wrote:
> The cursor handling in mgag200 is complicated to understand. It touches a
> number of different BOs, but doesn't really use all of them.
>
> Rewriting the cursor update reduces the amount of cursor state. There are
> two BOs for double-buffered HW updates. The source BO updates the one that
> is currently not displayed and then switches buffers. Explicit BO locking
> has been removed from the code. BOs are simply pinned and unpinned in video
> RAM.
>
> Signed-off-by: Thomas Zimmermann
> Acked-by: Gerd Hoffmann
> ---
> drivers/gpu/drm/mgag200/mgag200_cursor.c | 165 ++-
> drivers/gpu/drm/mgag200/mgag200_drv.h| 3 -
> drivers/gpu/drm/mgag200/mgag200_main.c | 4 +-
> 3 files changed, 69 insertions(+), 103 deletions(-)
>
> diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c
> b/drivers/gpu/drm/mgag200/mgag200_cursor.c
> index de94a650077b..5a22ef825588 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_cursor.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c
> @@ -19,10 +19,9 @@ static void mga_hide_cursor(struct mga_device *mdev)
> {
> WREG8(MGA_CURPOSXL, 0);
> WREG8(MGA_CURPOSXH, 0);
> - if (mdev->cursor.pixels_1->pin_count)
> - drm_gem_vram_unpin_locked(mdev->cursor.pixels_1);
> - if (mdev->cursor.pixels_2->pin_count)
> - drm_gem_vram_unpin_locked(mdev->cursor.pixels_2);
> + if (mdev->cursor.pixels_current)
> + drm_gem_vram_unpin(mdev->cursor.pixels_current);
> + mdev->cursor.pixels_current = NULL;
> }
>
> int mga_crtc_cursor_set(struct drm_crtc *crtc,
> @@ -36,7 +35,7 @@ int mga_crtc_cursor_set(struct drm_crtc *crtc,
> struct drm_gem_vram_object *pixels_1 = mdev->cursor.pixels_1;
> struct drm_gem_vram_object *pixels_2 = mdev->cursor.pixels_2;
> struct drm_gem_vram_object *pixels_current =
> mdev->cursor.pixels_current;
> - struct drm_gem_vram_object *pixels_prev = mdev->cursor.pixels_prev;
> + struct drm_gem_vram_object *pixels_next;
> struct drm_gem_object *obj;
> struct drm_gem_vram_object *gbo = NULL;
> int ret = 0;
> @@ -49,6 +48,7 @@ int mga_crtc_cursor_set(struct drm_crtc *crtc,
> bool found = false;
> int colour_count = 0;
> s64 gpu_addr;
> + u64 dst_gpu;
> u8 reg_index;
> u8 this_row[48];
>
> @@ -58,80 +58,66 @@ int mga_crtc_cursor_set(struct drm_crtc *crtc,
> return -ENOTSUPP; /* Didn't allocate space for cursors */
> }
>
> - if ((width != 64 || height != 64) && handle) {
> - WREG8(MGA_CURPOSXL, 0);
> - WREG8(MGA_CURPOSXH, 0);
> - return -EINVAL;
> + if (WARN_ON(pixels_current &&
> + pixels_1 != pixels_current &&
> + pixels_2 != pixels_current)) {
> + return -ENOTSUPP; /* inconsistent state */
> }
>
> - BUG_ON(pixels_1 != pixels_current && pixels_1 != pixels_prev);
> - BUG_ON(pixels_2 != pixels_current && pixels_2 != pixels_prev);
> - BUG_ON(pixels_current == pixels_prev);
> -
> if (!handle || !file_priv) {
> mga_hide_cursor(mdev);
> return 0;
> }
>
> - obj = drm_gem_object_lookup(file_priv, handle);
> - if (!obj)
> - return -ENOENT;
> -
> - ret = drm_gem_vram_lock(pixels_1, true);
> - if (ret) {
> + if (width != 64 || height != 64) {
> WREG8(MGA_CURPOSXL, 0);
> WREG8(MGA_CURPOSXH, 0);
> - goto out_unref;
> - }
> - ret = drm_gem_vram_lock(pixels_2, true);
> - if (ret) {
> - WREG8(MGA_CURPOSXL, 0);
> - WREG8(MGA_CURPOSXH, 0);
> - drm_gem_vram_unlock(pixels_1);
> - goto out_unlock1;
> + return -EINVAL;
> }
>
> - /* Move cursor buffers into VRAM if they aren't already */
> - if (!pixels_1->pin_count) {
> - ret = drm_gem_vram_pin_locked(pixels_1,
> - DRM_GEM_VRAM_PL_FLAG_VRAM);
> - if (ret)
> - goto out1;
> - gpu_addr = drm_gem_vram_offset(pixels_1);
> - if (gpu_addr < 0) {
> - drm_gem_vram_unpin_locked(pixels_1);
> - goto out1;
> - }
> - mdev->cursor.pixels_1_gpu_addr = gpu_addr;
> - }
> - if (!pixels_2->pin_count) {
> - ret = drm_gem_vram_pin_locked(pixels_2,
> - DRM_GEM_VRAM_PL_FLAG_VRAM);
> - if (ret) {
> - drm_gem_vram_unpin_locked(pixels_1);
> - goto out1;
> - }
> - gpu_addr = drm_gem_vram_offset(pixels_2);
> - if (gpu_addr < 0) {
> - drm_gem_vram_unpin_locked(pixels_1);
> - drm_gem_vram_unpin_locked(pixels_2);
> - goto out1;
> - }
Re: [PATCH] dma-buf: refcount the attachment for cache_sgt_mapping
Am 12.06.19 um 10:15 schrieb Nicolin Chen: > Hi Christian, > > On Wed, Jun 12, 2019 at 08:05:53AM +, Koenig, Christian wrote: >> Am 12.06.19 um 10:02 schrieb Nicolin Chen: >> [SNIP] >>> We haven't used DRM/GRM_PRIME yet but I am also curious would it >>> benefit DRM also if we reduce this overhead in the dma_buf? >> No, not at all. > From you replies, in a summary, does it means that there won't be a case > of DRM having a dma_buf attaching to the same device, i.e. multiple calls > of drm_gem_prime_import() function with same parameters of dev + dma_buf? Well, there are some cases where this happens. But in those cases we intentionally want to get a new attachment :) So thinking more about it you would actually break those and that is not something we can do. > If so, we can just ignore/drop this patch. Sorry for the misunderstanding. It might be interesting for things like P2P, but even then it might be better to just cache the P2P settings instead of the full attachment. Regards, Christian. > > Thanks > Nicolin
Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
On 12-06-19, 13:12, Rajendra Nayak wrote:
> so the 'fmax' tables basically say what the max frequency the device can
> operate at for a given performance state/voltage level.
>
> so in your example it would be for instance
>
> 500M, Perf state = 2
> 1G, Perf state = 3
> 1.2G, Perf state = 4
>
> Now when the device wants to operate at say 800Mhz, you need to set the
> Perf state to 3, so this patch basically avoids you having to put those
> additional
> OPPs in the table which would otherwise look something like this
>
> 500M, Perf state = 2
> 800M, Perf state = 3 <-- redundant OPP
> 1G, Perf state = 3
> 1.2G, Perf state = 4
>
> Your example had just 1 missing entry in the 'fmax' tables in reality its a
> lot more,
> atleast on all qualcomm platforms.
Okay, I have applied this patch (alone) to the OPP tree with minor
modifications in commit log and diff.
--
viresh
-8<-
From: Stephen Boyd
Date: Wed, 20 Mar 2019 15:19:08 +0530
Subject: [PATCH] opp: Don't overwrite rounded clk rate
Doing this allows us to call this API with any rate requested and have
it no need to match in the OPP table. Instead, we'll round the rate up
to the nearest OPP, so that we can get the voltage or performance level
required for that OPP. This supports users of the OPP core that want to
specify the possible 'fmax' values corresponding to the voltage or
performance levels of each OPP. And for devices that required the exact
frequency, we can rely on the clk framework to round the rate to the
nearest supported frequency instead of the OPP framework to do so.
Signed-off-by: Stephen Boyd
Signed-off-by: Rajendra Nayak
[ Viresh: Massaged changelog and use temp_opp variable instead ]
Signed-off-by: Viresh Kumar
---
drivers/opp/core.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 764e05a2fa66..0fbc77f05048 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -757,7 +757,7 @@ static int _set_required_opps(struct device *dev,
int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
{
struct opp_table *opp_table;
- unsigned long freq, old_freq;
+ unsigned long freq, old_freq, temp_freq;
struct dev_pm_opp *old_opp, *opp;
struct clk *clk;
int ret;
@@ -796,13 +796,15 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long
target_freq)
goto put_opp_table;
}
- old_opp = _find_freq_ceil(opp_table, &old_freq);
+ temp_freq = old_freq;
+ old_opp = _find_freq_ceil(opp_table, &temp_freq);
if (IS_ERR(old_opp)) {
dev_err(dev, "%s: failed to find current OPP for freq %lu
(%ld)\n",
__func__, old_freq, PTR_ERR(old_opp));
}
- opp = _find_freq_ceil(opp_table, &freq);
+ temp_freq = freq;
+ opp = _find_freq_ceil(opp_table, &temp_freq);
if (IS_ERR(opp)) {
ret = PTR_ERR(opp);
dev_err(dev, "%s: failed to find OPP for freq %lu (%d)\n",
Re: [PATCH] dma-buf: refcount the attachment for cache_sgt_mapping
On Wed, Jun 12, 2019 at 08:20:41AM +, Koenig, Christian wrote: > Am 12.06.19 um 10:15 schrieb Nicolin Chen: > > Hi Christian, > > > > On Wed, Jun 12, 2019 at 08:05:53AM +, Koenig, Christian wrote: > >> Am 12.06.19 um 10:02 schrieb Nicolin Chen: > >> [SNIP] > >>> We haven't used DRM/GRM_PRIME yet but I am also curious would it > >>> benefit DRM also if we reduce this overhead in the dma_buf? > >> No, not at all. > > From you replies, in a summary, does it means that there won't be a case > > of DRM having a dma_buf attaching to the same device, i.e. multiple calls > > of drm_gem_prime_import() function with same parameters of dev + dma_buf? > > Well, there are some cases where this happens. But in those cases we > intentionally want to get a new attachment :) Got it. > So thinking more about it you would actually break those and that is not > something we can do. That's true... > > If so, we can just ignore/drop this patch. Sorry for the misunderstanding. > > It might be interesting for things like P2P, but even then it might be > better to just cache the P2P settings instead of the full attachment. I see. Thank you for the answers!
Re: [PATCH v2 1/9] drm/gem-vram: Support pinning buffers to current location
Hi
Am 12.06.19 um 10:13 schrieb Gerd Hoffmann:
> On Tue, Jun 11, 2019 at 03:03:36PM +0200, Thomas Zimmermann wrote:
>> Pinning a buffer prevents it from being moved to a different memory
>> location. For some operations, such as buffer updates, it is not
>> important where the buffer is located. Setting the pin function's
>> pl_flag argument to 0 will pin the buffer to whereever it is stored.
>>
>> Signed-off-by: Thomas Zimmermann
>> ---
>> drivers/gpu/drm/drm_gem_vram_helper.c | 12
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
>> b/drivers/gpu/drm/drm_gem_vram_helper.c
>> index 42ad80888df7..214f54b8920b 100644
>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>> @@ -224,7 +224,9 @@ EXPORT_SYMBOL(drm_gem_vram_offset);
>> *
>> * Pinning a buffer object ensures that it is not evicted from
>> * a memory region. A pinned buffer object has to be unpinned before
>> - * it can be pinned to another region.
>> + * it can be pinned to another region. If the pl_flag argument is 0,
>> + * the buffer is pinned at its current location (video RAM or system
>> + * memory).
>> *
>> * Returns:
>> * 0 on success, or
>> @@ -242,7 +244,9 @@ int drm_gem_vram_pin(struct drm_gem_vram_object *gbo,
>> unsigned long pl_flag)
>> if (gbo->pin_count)
>> goto out;
>>
>> -drm_gem_vram_placement(gbo, pl_flag);
>> +if (pl_flag)
>> +drm_gem_vram_placement(gbo, pl_flag);
>> +
>> for (i = 0; i < gbo->placement.num_placement; ++i)
>> gbo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT;
>>
>> @@ -691,7 +695,7 @@ int drm_gem_vram_driver_gem_prime_pin(struct
>> drm_gem_object *gem)
>> {
>> struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem);
>>
>> -return drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
>> +return drm_gem_vram_pin(gbo, 0);
The only use case for these Prime helpers is fbdev console emulation. I
have another patch set that replaces the ast and mgag200 consoles with
generic code. During the console updates it temporarily pins the BO via
this Prime funcation, which might move the BO into scarce VRAM
unnecessarily. Can we leave it like this and add a comment explaining
the decision?
Best regards
Thomas
> Not sure this is a good idea here. If the bo happens to be in sysram
> it can't be displayed any more.
>
>> -ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
>> +ret = drm_gem_vram_pin(gbo, 0);
>
> Likewise.
>
> cheers,
> Gerd
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 00/15] tc358767 driver improvements
Everyone: This series contains various improvements (at least in my mind) and fixes that I made to tc358767 while working with the code of the driver. Hopefuly each patch is self explanatory. Feedback is welcome! Thanks, Andrey Smirnov Changes since [v4]: - tc_pllupdate_pllen() renamed to tc_pllupdate() - Collected Reviewed-bys from Andrzej for the rest of the series Changes since [v3]: - Collected Reviewed-bys from Andrzej - Dropped explicit check for -ETIMEDOUT in "drm/bridge: tc358767: Simplify polling in tc_main_link_setup()" for consistency - AUX transfer code converted to user regmap_raw_read(), regmap_raw_write() Changes since [v2]: - Patchset rebased on top of v4 of Tomi's series that recently went in (https://patchwork.freedesktop.org/series/58176/#rev5) - AUX transfer code converted to user regmap_bulk_read(), regmap_bulk_write() Changes since [v1]: - Patchset rebased on top of https://patchwork.freedesktop.org/series/58176/ - Patches to remove both tc_write() and tc_read() helpers added - Patches to rework AUX transfer code added - Both "drm/bridge: tc358767: Simplify polling in tc_main_link_setup()" and "drm/bridge: tc358767: Simplify polling in tc_link_training()" changed to use tc_poll_timeout() instead of regmap_read_poll_timeout() [v4] lkml.kernel.org/r/20190607044550.13361-1-andrew.smir...@gmail.com [v3] lkml.kernel.org/r/20190605070507.11417-1-andrew.smir...@gmail.com [v2] lkml.kernel.org/r/20190322032901.12045-1-andrew.smir...@gmail.com [v1] lkml.kernel.org/r/20190226193609.9862-1-andrew.smir...@gmail.com Andrey Smirnov (15): drm/bridge: tc358767: Simplify tc_poll_timeout() drm/bridge: tc358767: Simplify polling in tc_main_link_setup() drm/bridge: tc358767: Simplify polling in tc_link_training() drm/bridge: tc358767: Simplify tc_set_video_mode() drm/bridge: tc358767: Drop custom tc_write()/tc_read() accessors drm/bridge: tc358767: Simplify AUX data read drm/bridge: tc358767: Simplify AUX data write drm/bridge: tc358767: Increase AUX transfer length limit drm/bridge: tc358767: Use reported AUX transfer size drm/bridge: tc358767: Add support for address-only I2C transfers drm/bridge: tc358767: Introduce tc_set_syspllparam() drm/bridge: tc358767: Introduce tc_pllupdate() drm/bridge: tc358767: Simplify tc_aux_wait_busy() drm/bridge: tc358767: Drop unnecessary 8 byte buffer drm/bridge: tc358767: Replace magic number in tc_main_link_enable() drivers/gpu/drm/bridge/tc358767.c | 648 +- 1 file changed, 372 insertions(+), 276 deletions(-) -- 2.21.0
[PATCH v5 02/15] drm/bridge: tc358767: Simplify polling in tc_main_link_setup()
Replace explicit polling loop with equivalent call to
tc_poll_timeout() for brevity. No functional change intended.
Signed-off-by: Andrey Smirnov
Reviewed-by: Andrzej Hajda
Cc: Andrzej Hajda
Cc: Laurent Pinchart
Cc: Tomi Valkeinen
Cc: Andrey Gusakov
Cc: Philipp Zabel
Cc: Cory Tusar
Cc: Chris Healy
Cc: Lucas Stach
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
---
drivers/gpu/drm/bridge/tc358767.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c
b/drivers/gpu/drm/bridge/tc358767.c
index fb8a1942ec54..f463ef6d4271 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -774,7 +774,6 @@ static int tc_main_link_enable(struct tc_data *tc)
struct device *dev = tc->dev;
unsigned int rate;
u32 dp_phy_ctrl;
- int timeout;
u32 value;
int ret;
u8 tmp[8];
@@ -831,15 +830,10 @@ static int tc_main_link_enable(struct tc_data *tc)
dp_phy_ctrl &= ~(DP_PHY_RST | PHY_M1_RST | PHY_M0_RST);
tc_write(DP_PHY_CTRL, dp_phy_ctrl);
- timeout = 1000;
- do {
- tc_read(DP_PHY_CTRL, &value);
- udelay(1);
- } while ((!(value & PHY_RDY)) && (--timeout));
-
- if (timeout == 0) {
+ ret = tc_poll_timeout(tc, DP_PHY_CTRL, PHY_RDY, PHY_RDY, 1, 1000);
+ if (ret) {
dev_err(dev, "timeout waiting for phy become ready");
- return -ETIMEDOUT;
+ return ret;
}
/* Set misc: 8 bits per color */
--
2.21.0
[PATCH v5 10/15] drm/bridge: tc358767: Add support for address-only I2C transfers
Transfer size of zero means a request to do an address-only
transfer. Since the HW support this, we probably shouldn't be just
ignoring such requests. While at it allow DP_AUX_I2C_MOT flag to pass
through, since it is supported by the HW as well.
Signed-off-by: Andrey Smirnov
Reviewed-by: Andrzej Hajda
Cc: Andrzej Hajda
Cc: Laurent Pinchart
Cc: Tomi Valkeinen
Cc: Andrey Gusakov
Cc: Philipp Zabel
Cc: Cory Tusar
Cc: Chris Healy
Cc: Lucas Stach
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
---
drivers/gpu/drm/bridge/tc358767.c | 30 +++---
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c
b/drivers/gpu/drm/bridge/tc358767.c
index 7d0fbb12195b..4bb9b15e1324 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -145,6 +145,8 @@
/* AUX channel */
#define DP0_AUXCFG00x0660
+#define DP0_AUXCFG0_BSIZE GENMASK(11, 8)
+#define DP0_AUXCFG0_ADDR_ONLY BIT(4)
#define DP0_AUXCFG10x0664
#define AUX_RX_FILTER_EN BIT(16)
@@ -327,6 +329,18 @@ static int tc_aux_read_data(struct tc_data *tc, void
*data, size_t size)
return size;
}
+static u32 tc_auxcfg0(struct drm_dp_aux_msg *msg, size_t size)
+{
+ u32 auxcfg0 = msg->request;
+
+ if (size)
+ auxcfg0 |= FIELD_PREP(DP0_AUXCFG0_BSIZE, size - 1);
+ else
+ auxcfg0 |= DP0_AUXCFG0_ADDR_ONLY;
+
+ return auxcfg0;
+}
+
static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
{
@@ -336,9 +350,6 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
u32 auxstatus;
int ret;
- if (size == 0)
- return 0;
-
ret = tc_aux_wait_busy(tc, 100);
if (ret)
return ret;
@@ -362,8 +373,7 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
if (ret)
return ret;
/* Start transfer */
- ret = regmap_write(tc->regmap, DP0_AUXCFG0,
- ((size - 1) << 8) | request);
+ ret = regmap_write(tc->regmap, DP0_AUXCFG0, tc_auxcfg0(msg, size));
if (ret)
return ret;
@@ -377,8 +387,14 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
if (auxstatus & AUX_TIMEOUT)
return -ETIMEDOUT;
-
- size = FIELD_GET(AUX_BYTES, auxstatus);
+ /*
+* For some reason address-only DP_AUX_I2C_WRITE (MOT), still
+* reports 1 byte transferred in its status. To deal we that
+* we ignore aux_bytes field if we know that this was an
+* address-only transfer
+*/
+ if (size)
+ size = FIELD_GET(AUX_BYTES, auxstatus);
msg->reply = FIELD_GET(AUX_STATUS, auxstatus);
switch (request) {
--
2.21.0
[PATCH v5 03/15] drm/bridge: tc358767: Simplify polling in tc_link_training()
Replace explicit polling in tc_link_training() with equivalent call to
tc_poll_timeout() for simplicity. No functional change intended (not
including slightly altered debug output).
Signed-off-by: Andrey Smirnov
Reviewed-by: Andrzej Hajda
Cc: Andrzej Hajda
Cc: Laurent Pinchart
Cc: Tomi Valkeinen
Cc: Andrey Gusakov
Cc: Philipp Zabel
Cc: Cory Tusar
Cc: Chris Healy
Cc: Lucas Stach
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
---
drivers/gpu/drm/bridge/tc358767.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c
b/drivers/gpu/drm/bridge/tc358767.c
index f463ef6d4271..31f5045e7e42 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -748,22 +748,19 @@ static int tc_set_video_mode(struct tc_data *tc,
static int tc_wait_link_training(struct tc_data *tc)
{
- u32 timeout = 1000;
u32 value;
int ret;
- do {
- udelay(1);
- tc_read(DP0_LTSTAT, &value);
- } while ((!(value & LT_LOOPDONE)) && (--timeout));
-
- if (timeout == 0) {
+ ret = tc_poll_timeout(tc, DP0_LTSTAT, LT_LOOPDONE,
+ LT_LOOPDONE, 1, 1000);
+ if (ret) {
dev_err(tc->dev, "Link training timeout waiting for
LT_LOOPDONE!\n");
- return -ETIMEDOUT;
+ return ret;
}
- return (value >> 8) & 0x7;
+ tc_read(DP0_LTSTAT, &value);
+ return (value >> 8) & 0x7;
err:
return ret;
}
--
2.21.0
[PATCH v5 04/15] drm/bridge: tc358767: Simplify tc_set_video_mode()
Simplify tc_set_video_mode() by replacing explicit shifting using macros from . No functional change intended. Signed-off-by: Andrey Smirnov Reviewed-by: Andrzej Hajda Cc: Andrzej Hajda Cc: Laurent Pinchart Cc: Tomi Valkeinen Cc: Andrey Gusakov Cc: Philipp Zabel Cc: Cory Tusar Cc: Chris Healy Cc: Lucas Stach Cc: dri-devel@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org --- drivers/gpu/drm/bridge/tc358767.c | 106 ++ 1 file changed, 78 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index 31f5045e7e42..5b78021d6c5b 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -24,6 +24,7 @@ * GNU General Public License for more details. */ +#include #include #include #include @@ -56,6 +57,7 @@ /* Video Path */ #define VPCTRL00x0450 +#define VSDELAYGENMASK(31, 20) #define OPXLFMT_RGB666 (0 << 8) #define OPXLFMT_RGB888 (1 << 8) #define FRMSYNC_DISABLED (0 << 4) /* Video Timing Gen Disabled */ @@ -63,9 +65,17 @@ #define MSF_DISABLED (0 << 0) /* Magic Square FRC disabled */ #define MSF_ENABLED(1 << 0) /* Magic Square FRC enabled */ #define HTIM01 0x0454 +#define HPWGENMASK(8, 0) +#define HBPR GENMASK(24, 16) #define HTIM02 0x0458 +#define HDISPR GENMASK(10, 0) +#define HFPR GENMASK(24, 16) #define VTIM01 0x045c +#define VSPR GENMASK(7, 0) +#define VBPR GENMASK(23, 16) #define VTIM02 0x0460 +#define VFPR GENMASK(23, 16) +#define VDISPR GENMASK(10, 0) #define VFUEN0 0x0464 #define VFUEN BIT(0) /* Video Frame Timing Upload */ @@ -108,14 +118,28 @@ /* Main Channel */ #define DP0_SECSAMPLE 0x0640 #define DP0_VIDSYNCDELAY 0x0644 +#define VID_SYNC_DLY GENMASK(15, 0) +#define THRESH_DLY GENMASK(31, 16) + #define DP0_TOTALVAL 0x0648 +#define H_TOTALGENMASK(15, 0) +#define V_TOTALGENMASK(31, 16) #define DP0_STARTVAL 0x064c +#define H_STARTGENMASK(15, 0) +#define V_STARTGENMASK(31, 16) #define DP0_ACTIVEVAL 0x0650 +#define H_ACT GENMASK(15, 0) +#define V_ACT GENMASK(31, 16) + #define DP0_SYNCVAL0x0654 +#define VS_WIDTH GENMASK(30, 16) +#define HS_WIDTH GENMASK(14, 0) #define SYNCVAL_HS_POL_ACTIVE_LOW (1 << 15) #define SYNCVAL_VS_POL_ACTIVE_LOW (1 << 31) #define DP0_MISC 0x0658 #define TU_SIZE_RECOMMENDED(63) /* LSCLK cycles per TU */ +#define MAX_TU_SYMBOL GENMASK(28, 23) +#define TU_SIZEGENMASK(21, 16) #define BPC_6 (0 << 5) #define BPC_8 (1 << 5) @@ -192,6 +216,12 @@ /* Test & Debug */ #define TSTCTL 0x0a00 +#define COLOR_RGENMASK(31, 24) +#define COLOR_GGENMASK(23, 16) +#define COLOR_BGENMASK(15, 8) +#define ENI2CFILTERBIT(4) +#define COLOR_BAR_MODE GENMASK(1, 0) +#define COLOR_BAR_MODE_BARS2 #define PLL_DBG0x0a04 static bool tc_test_pattern; @@ -672,6 +702,7 @@ static int tc_set_video_mode(struct tc_data *tc, int upper_margin = mode->vtotal - mode->vsync_end; int lower_margin = mode->vsync_start - mode->vdisplay; int vsync_len = mode->vsync_end - mode->vsync_start; + u32 dp0_syncval; /* * Recommended maximum number of symbols transferred in a transfer unit: @@ -696,50 +727,69 @@ static int tc_set_video_mode(struct tc_data *tc, * assume we do not need any delay when DPI is a source of * sync signals */ - tc_write(VPCTRL0, (0 << 20) /* VSDELAY */ | + tc_write(VPCTRL0, +FIELD_PREP(VSDELAY, 0) | OPXLFMT_RGB888 | FRMSYNC_DISABLED | MSF_DISABLED); - tc_write(HTIM01, (ALIGN(left_margin, 2) << 16) | /* H back porch */ -(ALIGN(hsync_len, 2) << 0));/* Hsync */ - tc_write(HTIM02, (ALIGN(right_margin, 2) << 16) | /* H front porch */ -(ALIGN(mode->hdisplay, 2) << 0)); /* width */ - tc_write(VTIM01, (upper_margin << 16) | /* V back porch */ -(vsync_len << 0)); /* Vsync */ - tc_write(VTIM02, (lower_margin << 16) | /* V front porch */ -(mode->vdisplay << 0));/* height */ + tc_write(HTIM01, +
[PATCH v5 12/15] drm/bridge: tc358767: Introduce tc_pllupdate()
tc_wait_pll_lock() is always called as a follow-up for updating
PLLUPDATE and PLLEN bit of a given PLL control register. To simplify
things, merge the two operation into a single helper function
tc_pllupdate() and convert the rest of the code to use it. No
functional change intended.
Signed-off-by: Andrey Smirnov
Reviewed-by: Laurent Pinchart
Reviewed-by: Andrzej Hajda
Cc: Andrzej Hajda
Cc: Laurent Pinchart
Cc: Tomi Valkeinen
Cc: Andrey Gusakov
Cc: Philipp Zabel
Cc: Cory Tusar
Cc: Chris Healy
Cc: Lucas Stach
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
---
drivers/gpu/drm/bridge/tc358767.c | 30 ++
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c
b/drivers/gpu/drm/bridge/tc358767.c
index ac55b59249e3..28df53f7c349 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -443,10 +443,18 @@ static u32 tc_srcctrl(struct tc_data *tc)
return reg;
}
-static void tc_wait_pll_lock(struct tc_data *tc)
+static int tc_pllupdate(struct tc_data *tc, unsigned int pllctrl)
{
+ int ret;
+
+ ret = regmap_write(tc->regmap, pllctrl, PLLUPDATE | PLLEN);
+ if (ret)
+ return ret;
+
/* Wait for PLL to lock: up to 2.09 ms, depending on refclk */
usleep_range(3000, 6000);
+
+ return 0;
}
static int tc_pxl_pll_en(struct tc_data *tc, u32 refclk, u32 pixelclock)
@@ -546,13 +554,7 @@ static int tc_pxl_pll_en(struct tc_data *tc, u32 refclk,
u32 pixelclock)
return ret;
/* Force PLL parameter update and disable bypass */
- ret = regmap_write(tc->regmap, PXL_PLLCTRL, PLLUPDATE | PLLEN);
- if (ret)
- return ret;
-
- tc_wait_pll_lock(tc);
-
- return 0;
+ return tc_pllupdate(tc, PXL_PLLCTRL);
}
static int tc_pxl_pll_dis(struct tc_data *tc)
@@ -626,15 +628,13 @@ static int tc_aux_link_setup(struct tc_data *tc)
* Initially PLLs are in bypass. Force PLL parameter update,
* disable PLL bypass, enable PLL
*/
- ret = regmap_write(tc->regmap, DP0_PLLCTRL, PLLUPDATE | PLLEN);
+ ret = tc_pllupdate(tc, DP0_PLLCTRL);
if (ret)
goto err;
- tc_wait_pll_lock(tc);
- ret = regmap_write(tc->regmap, DP1_PLLCTRL, PLLUPDATE | PLLEN);
+ ret = tc_pllupdate(tc, DP1_PLLCTRL);
if (ret)
goto err;
- tc_wait_pll_lock(tc);
ret = tc_poll_timeout(tc, DP_PHY_CTRL, PHY_RDY, PHY_RDY, 1, 1000);
if (ret == -ETIMEDOUT) {
@@ -914,15 +914,13 @@ static int tc_main_link_enable(struct tc_data *tc)
return ret;
/* PLL setup */
- ret = regmap_write(tc->regmap, DP0_PLLCTRL, PLLUPDATE | PLLEN);
+ ret = tc_pllupdate(tc, DP0_PLLCTRL);
if (ret)
return ret;
- tc_wait_pll_lock(tc);
- ret = regmap_write(tc->regmap, DP1_PLLCTRL, PLLUPDATE | PLLEN);
+ ret = tc_pllupdate(tc, DP1_PLLCTRL);
if (ret)
return ret;
- tc_wait_pll_lock(tc);
/* Reset/Enable Main Links */
dp_phy_ctrl |= DP_PHY_RST | PHY_M1_RST | PHY_M0_RST;
--
2.21.0
[PATCH v5 08/15] drm/bridge: tc358767: Increase AUX transfer length limit
According to the datasheet tc358767 can transfer up to 16 bytes via
its AUX channel, so the artificial limit of 8 appears to be too
low. However only up to 15-bytes seem to be actually supported and
trying to use 16-byte transfers results in transfers failing
sporadically (with bogus status in case of I2C transfers), so limit it
to 15.
Signed-off-by: Andrey Smirnov
Reviewed-by: Andrzej Hajda
Cc: Andrzej Hajda
Cc: Laurent Pinchart
Cc: Tomi Valkeinen
Cc: Andrey Gusakov
Cc: Philipp Zabel
Cc: Cory Tusar
Cc: Chris Healy
Cc: Lucas Stach
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
---
drivers/gpu/drm/bridge/tc358767.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c
b/drivers/gpu/drm/bridge/tc358767.c
index e60692b8cd69..8b53dc8908d3 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -354,7 +354,7 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
{
struct tc_data *tc = aux_to_tc(aux);
- size_t size = min_t(size_t, 8, msg->size);
+ size_t size = min_t(size_t, DP_AUX_MAX_PAYLOAD_BYTES - 1, msg->size);
u8 request = msg->request & ~DP_AUX_I2C_MOT;
int ret;
--
2.21.0
[PATCH v5 05/15] drm/bridge: tc358767: Drop custom tc_write()/tc_read() accessors
A very unfortunate aspect of tc_write()/tc_read() macro helpers is
that they capture quite a bit of context around them and thus require
the caller to have magic variables 'ret' and 'tc' as well as label
'err'. That makes a number of code paths rather counter-intuitive and
somewhat clunky, for example tc_stream_clock_calc() ends up being like
this:
int ret;
tc_write(DP0_VIDMNGEN1, 32768);
return 0;
err:
return ret;
which is rather surprising when you read the code for the first
time. Since those helpers arguably aren't really saving that much code
and there's no way of fixing them without making them too verbose to
be worth it change the driver code to not use them at all.
Signed-off-by: Andrey Smirnov
Reviewed-by: Andrzej Hajda
Cc: Andrzej Hajda
Cc: Laurent Pinchart
Cc: Tomi Valkeinen
Cc: Andrey Gusakov
Cc: Philipp Zabel
Cc: Cory Tusar
Cc: Chris Healy
Cc: Lucas Stach
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
---
drivers/gpu/drm/bridge/tc358767.c | 382 ++
1 file changed, 230 insertions(+), 152 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c
b/drivers/gpu/drm/bridge/tc358767.c
index 5b78021d6c5b..7b15caec2ce5 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -280,20 +280,6 @@ static inline struct tc_data *connector_to_tc(struct
drm_connector *c)
return container_of(c, struct tc_data, connector);
}
-/* Simple macros to avoid repeated error checks */
-#define tc_write(reg, var) \
- do {\
- ret = regmap_write(tc->regmap, reg, var); \
- if (ret)\
- goto err; \
- } while (0)
-#define tc_read(reg, var) \
- do {\
- ret = regmap_read(tc->regmap, reg, var);\
- if (ret)\
- goto err; \
- } while (0)
-
static inline int tc_poll_timeout(struct tc_data *tc, unsigned int addr,
unsigned int cond_mask,
unsigned int cond_value,
@@ -351,7 +337,7 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
ret = tc_aux_wait_busy(tc, 100);
if (ret)
- goto err;
+ return ret;
if (request == DP_AUX_I2C_WRITE || request == DP_AUX_NATIVE_WRITE) {
/* Store data */
@@ -362,7 +348,11 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
tmp = (tmp << 8) | buf[i];
i++;
if (((i % 4) == 0) || (i == size)) {
- tc_write(DP0_AUXWDATA((i - 1) >> 2), tmp);
+ ret = regmap_write(tc->regmap,
+ DP0_AUXWDATA((i - 1) >> 2),
+ tmp);
+ if (ret)
+ return ret;
tmp = 0;
}
}
@@ -372,23 +362,32 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
}
/* Store address */
- tc_write(DP0_AUXADDR, msg->address);
+ ret = regmap_write(tc->regmap, DP0_AUXADDR, msg->address);
+ if (ret)
+ return ret;
/* Start transfer */
- tc_write(DP0_AUXCFG0, ((size - 1) << 8) | request);
+ ret = regmap_write(tc->regmap, DP0_AUXCFG0,
+ ((size - 1) << 8) | request);
+ if (ret)
+ return ret;
ret = tc_aux_wait_busy(tc, 100);
if (ret)
- goto err;
+ return ret;
ret = tc_aux_get_status(tc, &msg->reply);
if (ret)
- goto err;
+ return ret;
if (request == DP_AUX_I2C_READ || request == DP_AUX_NATIVE_READ) {
/* Read data */
while (i < size) {
- if ((i % 4) == 0)
- tc_read(DP0_AUXRDATA(i >> 2), &tmp);
+ if ((i % 4) == 0) {
+ ret = regmap_read(tc->regmap,
+ DP0_AUXRDATA(i >> 2), &tmp);
+ if (ret)
+ return ret;
+ }
buf[i] = tmp & 0xff;
tmp = tmp >> 8;
i++;
@@ -396,8 +395,6 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
}
return size;
-err:
- return ret;
}
static const char * c
[PATCH v5 09/15] drm/bridge: tc358767: Use reported AUX transfer size
Don't assume that requested data transfer size is the same as amount
of data that was transferred. Change the code to get that information
from DP0_AUXSTATUS instead.
Since the check for AUX_BUSY in tc_aux_get_status() is pointless (it
will always called after tc_aux_wait_busy()) and there's only one user
of it, inline its code into tc_aux_transfer() instead of trying to
accommodate the change above.
Signed-off-by: Andrey Smirnov
Reviewed-by: Andrzej Hajda
Cc: Andrzej Hajda
Cc: Laurent Pinchart
Cc: Tomi Valkeinen
Cc: Andrey Gusakov
Cc: Philipp Zabel
Cc: Cory Tusar
Cc: Chris Healy
Cc: Lucas Stach
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
---
drivers/gpu/drm/bridge/tc358767.c | 40 ++-
1 file changed, 12 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c
b/drivers/gpu/drm/bridge/tc358767.c
index 8b53dc8908d3..7d0fbb12195b 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -152,10 +152,10 @@
#define DP0_AUXWDATA(i)(0x066c + (i) * 4)
#define DP0_AUXRDATA(i)(0x067c + (i) * 4)
#define DP0_AUXSTATUS 0x068c
-#define AUX_STATUS_MASK0xf0
-#define AUX_STATUS_SHIFT 4
-#define AUX_TIMEOUTBIT(1)
-#define AUX_BUSY BIT(0)
+#define AUX_BYTES GENMASK(15, 8)
+#define AUX_STATUS GENMASK(7, 4)
+#define AUX_TIMEOUTBIT(1)
+#define AUX_BUSY BIT(0)
#define DP0_AUXI2CADR 0x0698
/* Link Training */
@@ -298,29 +298,6 @@ static int tc_aux_wait_busy(struct tc_data *tc, unsigned
int timeout_ms)
1000, 1000 * timeout_ms);
}
-static int tc_aux_get_status(struct tc_data *tc, u8 *reply)
-{
- int ret;
- u32 value;
-
- ret = regmap_read(tc->regmap, DP0_AUXSTATUS, &value);
- if (ret < 0)
- return ret;
-
- if (value & AUX_BUSY) {
- dev_err(tc->dev, "aux busy!\n");
- return -EBUSY;
- }
-
- if (value & AUX_TIMEOUT) {
- dev_err(tc->dev, "aux access timeout!\n");
- return -ETIMEDOUT;
- }
-
- *reply = (value & AUX_STATUS_MASK) >> AUX_STATUS_SHIFT;
- return 0;
-}
-
static int tc_aux_write_data(struct tc_data *tc, const void *data,
size_t size)
{
@@ -356,6 +333,7 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
struct tc_data *tc = aux_to_tc(aux);
size_t size = min_t(size_t, DP_AUX_MAX_PAYLOAD_BYTES - 1, msg->size);
u8 request = msg->request & ~DP_AUX_I2C_MOT;
+ u32 auxstatus;
int ret;
if (size == 0)
@@ -393,10 +371,16 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
if (ret)
return ret;
- ret = tc_aux_get_status(tc, &msg->reply);
+ ret = regmap_read(tc->regmap, DP0_AUXSTATUS, &auxstatus);
if (ret)
return ret;
+ if (auxstatus & AUX_TIMEOUT)
+ return -ETIMEDOUT;
+
+ size = FIELD_GET(AUX_BYTES, auxstatus);
+ msg->reply = FIELD_GET(AUX_STATUS, auxstatus);
+
switch (request) {
case DP_AUX_NATIVE_READ:
case DP_AUX_I2C_READ:
--
2.21.0
[PATCH v5 07/15] drm/bridge: tc358767: Simplify AUX data write
Simplify AUX data write by dropping index arithmetic and shifting and
replacing it with a call to a helper function that does two things:
1. Copies user-provided data into a write buffer
2. Transfers contents of the write buffer to up to 4 32-bit
registers on the chip
Note that separate data endianness fix:
tmp = (tmp << 8) | buf[i];
that was reserved for DP_AUX_I2C_WRITE looks really strange, since it
will place data differently depending on the passed user-data
size. E.g. for a write of 1 byte, data transferred to the chip would
look like:
[byte0] [dummy1] [dummy2] [dummy3]
whereas for a write of 4 bytes we'd get:
[byte3] [byte2] [byte1] [byte0]
Since there's no indication in the datasheet that I2C write buffer
should be treated differently than AUX write buffer and no comment in
the original code explaining why it was done this way, that special
I2C write buffer transformation was dropped in this patch.
Signed-off-by: Andrey Smirnov
Reviewed-by: Andrzej Hajda
Cc: Andrzej Hajda
Cc: Laurent Pinchart
Cc: Tomi Valkeinen
Cc: Andrey Gusakov
Cc: Philipp Zabel
Cc: Cory Tusar
Cc: Chris Healy
Cc: Lucas Stach
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
---
drivers/gpu/drm/bridge/tc358767.c | 48 +--
1 file changed, 26 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c
b/drivers/gpu/drm/bridge/tc358767.c
index 7152b44db8a3..e60692b8cd69 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -321,6 +321,21 @@ static int tc_aux_get_status(struct tc_data *tc, u8 *reply)
return 0;
}
+static int tc_aux_write_data(struct tc_data *tc, const void *data,
+size_t size)
+{
+ u32 auxwdata[DP_AUX_MAX_PAYLOAD_BYTES / sizeof(u32)] = { 0 };
+ int ret, count = ALIGN(size, sizeof(u32));
+
+ memcpy(auxwdata, data, size);
+
+ ret = regmap_raw_write(tc->regmap, DP0_AUXWDATA(0), auxwdata, count);
+ if (ret)
+ return ret;
+
+ return size;
+}
+
static int tc_aux_read_data(struct tc_data *tc, void *data, size_t size)
{
u32 auxrdata[DP_AUX_MAX_PAYLOAD_BYTES / sizeof(u32)];
@@ -341,9 +356,6 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
struct tc_data *tc = aux_to_tc(aux);
size_t size = min_t(size_t, 8, msg->size);
u8 request = msg->request & ~DP_AUX_I2C_MOT;
- u8 *buf = msg->buffer;
- u32 tmp = 0;
- int i = 0;
int ret;
if (size == 0)
@@ -353,25 +365,17 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
if (ret)
return ret;
- if (request == DP_AUX_I2C_WRITE || request == DP_AUX_NATIVE_WRITE) {
- /* Store data */
- while (i < size) {
- if (request == DP_AUX_NATIVE_WRITE)
- tmp = tmp | (buf[i] << (8 * (i & 0x3)));
- else
- tmp = (tmp << 8) | buf[i];
- i++;
- if (((i % 4) == 0) || (i == size)) {
- ret = regmap_write(tc->regmap,
- DP0_AUXWDATA((i - 1) >> 2),
- tmp);
- if (ret)
- return ret;
- tmp = 0;
- }
- }
- } else if (request != DP_AUX_I2C_READ &&
- request != DP_AUX_NATIVE_READ) {
+ switch (request) {
+ case DP_AUX_NATIVE_READ:
+ case DP_AUX_I2C_READ:
+ break;
+ case DP_AUX_NATIVE_WRITE:
+ case DP_AUX_I2C_WRITE:
+ ret = tc_aux_write_data(tc, msg->buffer, size);
+ if (ret < 0)
+ return ret;
+ break;
+ default:
return -EINVAL;
}
--
2.21.0
[PATCH v5 14/15] drm/bridge: tc358767: Drop unnecessary 8 byte buffer
tc_get_display_props() never reads more than a byte via AUX, so
there's no need to reserve 8 for that purpose. No function change
intended.
Signed-off-by: Andrey Smirnov
Reviewed-by: Andrzej Hajda
Cc: Andrzej Hajda
Cc: Laurent Pinchart
Cc: Tomi Valkeinen
Cc: Andrey Gusakov
Cc: Philipp Zabel
Cc: Cory Tusar
Cc: Chris Healy
Cc: Lucas Stach
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
---
drivers/gpu/drm/bridge/tc358767.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c
b/drivers/gpu/drm/bridge/tc358767.c
index 01ca92a6d9c8..81ed359487c7 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -661,8 +661,7 @@ static int tc_aux_link_setup(struct tc_data *tc)
static int tc_get_display_props(struct tc_data *tc)
{
int ret;
- /* temp buffer */
- u8 tmp[8];
+ u8 reg;
/* Read DP Rx Link Capability */
ret = drm_dp_link_probe(&tc->aux, &tc->link.base);
@@ -678,21 +677,21 @@ static int tc_get_display_props(struct tc_data *tc)
tc->link.base.num_lanes = 2;
}
- ret = drm_dp_dpcd_readb(&tc->aux, DP_MAX_DOWNSPREAD, tmp);
+ ret = drm_dp_dpcd_readb(&tc->aux, DP_MAX_DOWNSPREAD, ®);
if (ret < 0)
goto err_dpcd_read;
- tc->link.spread = tmp[0] & DP_MAX_DOWNSPREAD_0_5;
+ tc->link.spread = reg & DP_MAX_DOWNSPREAD_0_5;
- ret = drm_dp_dpcd_readb(&tc->aux, DP_MAIN_LINK_CHANNEL_CODING, tmp);
+ ret = drm_dp_dpcd_readb(&tc->aux, DP_MAIN_LINK_CHANNEL_CODING, ®);
if (ret < 0)
goto err_dpcd_read;
tc->link.scrambler_dis = false;
/* read assr */
- ret = drm_dp_dpcd_readb(&tc->aux, DP_EDP_CONFIGURATION_SET, tmp);
+ ret = drm_dp_dpcd_readb(&tc->aux, DP_EDP_CONFIGURATION_SET, ®);
if (ret < 0)
goto err_dpcd_read;
- tc->link.assr = tmp[0] & DP_ALTERNATE_SCRAMBLER_RESET_ENABLE;
+ tc->link.assr = reg & DP_ALTERNATE_SCRAMBLER_RESET_ENABLE;
dev_dbg(tc->dev, "DPCD rev: %d.%d, rate: %s, lanes: %d, framing: %s\n",
tc->link.base.revision >> 4, tc->link.base.revision & 0x0f,
--
2.21.0
[PATCH v5 13/15] drm/bridge: tc358767: Simplify tc_aux_wait_busy()
We never pass anything but 100 as timeout_ms to tc_aux_wait_busy(), so
we may as well hardcode that value and simplify function's signature.
Signed-off-by: Andrey Smirnov
Reviewed-by: Andrzej Hajda
Cc: Andrzej Hajda
Cc: Laurent Pinchart
Cc: Tomi Valkeinen
Cc: Andrey Gusakov
Cc: Philipp Zabel
Cc: Cory Tusar
Cc: Chris Healy
Cc: Lucas Stach
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
---
drivers/gpu/drm/bridge/tc358767.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c
b/drivers/gpu/drm/bridge/tc358767.c
index 28df53f7c349..01ca92a6d9c8 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -294,10 +294,9 @@ static inline int tc_poll_timeout(struct tc_data *tc,
unsigned int addr,
sleep_us, timeout_us);
}
-static int tc_aux_wait_busy(struct tc_data *tc, unsigned int timeout_ms)
+static int tc_aux_wait_busy(struct tc_data *tc)
{
- return tc_poll_timeout(tc, DP0_AUXSTATUS, AUX_BUSY, 0,
- 1000, 1000 * timeout_ms);
+ return tc_poll_timeout(tc, DP0_AUXSTATUS, AUX_BUSY, 0, 1000, 10);
}
static int tc_aux_write_data(struct tc_data *tc, const void *data,
@@ -350,7 +349,7 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
u32 auxstatus;
int ret;
- ret = tc_aux_wait_busy(tc, 100);
+ ret = tc_aux_wait_busy(tc);
if (ret)
return ret;
@@ -377,7 +376,7 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
if (ret)
return ret;
- ret = tc_aux_wait_busy(tc, 100);
+ ret = tc_aux_wait_busy(tc);
if (ret)
return ret;
--
2.21.0
[PATCH v5 06/15] drm/bridge: tc358767: Simplify AUX data read
Simplify AUX data read by removing index arithmetic and shifting with
a helper function that does two things:
1. Fetch data from up to 4 32-bit registers from the chip
2. Copy read data into user provided array.
Signed-off-by: Andrey Smirnov
Reviewed-by: Andrzej Hajda
Cc: Andrzej Hajda
Cc: Laurent Pinchart
Cc: Tomi Valkeinen
Cc: Andrey Gusakov
Cc: Philipp Zabel
Cc: Cory Tusar
Cc: Chris Healy
Cc: Lucas Stach
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
---
drivers/gpu/drm/bridge/tc358767.c | 31 ++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c
b/drivers/gpu/drm/bridge/tc358767.c
index 7b15caec2ce5..7152b44db8a3 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -321,6 +321,20 @@ static int tc_aux_get_status(struct tc_data *tc, u8 *reply)
return 0;
}
+static int tc_aux_read_data(struct tc_data *tc, void *data, size_t size)
+{
+ u32 auxrdata[DP_AUX_MAX_PAYLOAD_BYTES / sizeof(u32)];
+ int ret, count = ALIGN(size, sizeof(u32));
+
+ ret = regmap_raw_read(tc->regmap, DP0_AUXRDATA(0), auxrdata, count);
+ if (ret)
+ return ret;
+
+ memcpy(data, auxrdata, size);
+
+ return size;
+}
+
static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
{
@@ -379,19 +393,10 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
if (ret)
return ret;
- if (request == DP_AUX_I2C_READ || request == DP_AUX_NATIVE_READ) {
- /* Read data */
- while (i < size) {
- if ((i % 4) == 0) {
- ret = regmap_read(tc->regmap,
- DP0_AUXRDATA(i >> 2), &tmp);
- if (ret)
- return ret;
- }
- buf[i] = tmp & 0xff;
- tmp = tmp >> 8;
- i++;
- }
+ switch (request) {
+ case DP_AUX_NATIVE_READ:
+ case DP_AUX_I2C_READ:
+ return tc_aux_read_data(tc, msg->buffer, size);
}
return size;
--
2.21.0
[PATCH v5 15/15] drm/bridge: tc358767: Replace magic number in tc_main_link_enable()
We don't need 8 byte array, DP_LINK_STATUS_SIZE (6) should be enough. This also gets rid of a magic number as a bonus. Signed-off-by: Andrey Smirnov Reviewed-by: Andrzej Hajda Cc: Andrzej Hajda Cc: Laurent Pinchart Cc: Tomi Valkeinen Cc: Andrey Gusakov Cc: Philipp Zabel Cc: Cory Tusar Cc: Chris Healy Cc: Lucas Stach Cc: dri-devel@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org --- drivers/gpu/drm/bridge/tc358767.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index 81ed359487c7..b403c7bad156 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -874,7 +874,7 @@ static int tc_main_link_enable(struct tc_data *tc) u32 dp_phy_ctrl; u32 value; int ret; - u8 tmp[8]; + u8 tmp[DP_LINK_STATUS_SIZE]; dev_dbg(tc->dev, "link enable\n"); -- 2.21.0
[PATCH v5 11/15] drm/bridge: tc358767: Introduce tc_set_syspllparam()
Move common code converting clock rate to an appropriate constant and
configuring SYS_PLLPARAM register into a separate routine and convert
the rest of the code to use it. No functional change intended.
Signed-off-by: Andrey Smirnov
Reviewed-by: Laurent Pinchart
Reviewed-by: Andrzej Hajda
Cc: Andrzej Hajda
Cc: Laurent Pinchart
Cc: Tomi Valkeinen
Cc: Andrey Gusakov
Cc: Philipp Zabel
Cc: Chris Healy
Cc: Cory Tusar
Cc: Lucas Stach
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
---
drivers/gpu/drm/bridge/tc358767.c | 46 +++
1 file changed, 16 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c
b/drivers/gpu/drm/bridge/tc358767.c
index 4bb9b15e1324..ac55b59249e3 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -581,35 +581,40 @@ static int tc_stream_clock_calc(struct tc_data *tc)
return regmap_write(tc->regmap, DP0_VIDMNGEN1, 32768);
}
-static int tc_aux_link_setup(struct tc_data *tc)
+static int tc_set_syspllparam(struct tc_data *tc)
{
unsigned long rate;
- u32 dp0_auxcfg1;
- u32 value;
- int ret;
+ u32 pllparam = SYSCLK_SEL_LSCLK | LSCLK_DIV_2;
rate = clk_get_rate(tc->refclk);
switch (rate) {
case 3840:
- value = REF_FREQ_38M4;
+ pllparam |= REF_FREQ_38M4;
break;
case 2600:
- value = REF_FREQ_26M;
+ pllparam |= REF_FREQ_26M;
break;
case 1920:
- value = REF_FREQ_19M2;
+ pllparam |= REF_FREQ_19M2;
break;
case 1300:
- value = REF_FREQ_13M;
+ pllparam |= REF_FREQ_13M;
break;
default:
dev_err(tc->dev, "Invalid refclk rate: %lu Hz\n", rate);
return -EINVAL;
}
+ return regmap_write(tc->regmap, SYS_PLLPARAM, pllparam);
+}
+
+static int tc_aux_link_setup(struct tc_data *tc)
+{
+ int ret;
+ u32 dp0_auxcfg1;
+
/* Setup DP-PHY / PLL */
- value |= SYSCLK_SEL_LSCLK | LSCLK_DIV_2;
- ret = regmap_write(tc->regmap, SYS_PLLPARAM, value);
+ ret = tc_set_syspllparam(tc);
if (ret)
goto err;
@@ -868,7 +873,6 @@ static int tc_main_link_enable(struct tc_data *tc)
{
struct drm_dp_aux *aux = &tc->aux;
struct device *dev = tc->dev;
- unsigned int rate;
u32 dp_phy_ctrl;
u32 value;
int ret;
@@ -896,25 +900,7 @@ static int tc_main_link_enable(struct tc_data *tc)
if (ret)
return ret;
- rate = clk_get_rate(tc->refclk);
- switch (rate) {
- case 3840:
- value = REF_FREQ_38M4;
- break;
- case 2600:
- value = REF_FREQ_26M;
- break;
- case 1920:
- value = REF_FREQ_19M2;
- break;
- case 1300:
- value = REF_FREQ_13M;
- break;
- default:
- return -EINVAL;
- }
- value |= SYSCLK_SEL_LSCLK | LSCLK_DIV_2;
- ret = regmap_write(tc->regmap, SYS_PLLPARAM, value);
+ ret = tc_set_syspllparam(tc);
if (ret)
return ret;
--
2.21.0
[PATCH] drm/bridge: dw-hdmi: Use automatic CTS generation mode when using non-AHB audio
When using an I2S source using a different clock source (usually the I2S
audio HW uses dedicated PLLs, different from the HDMI PHY PLL), fixed
CTS values will cause some frequent audio drop-out and glitches as
reported on Amlogic, Allwinner and Rockchip SoCs setups.
Setting the CTS in automatic mode will let the HDMI controller generate
automatically the CTS value to match the input audio clock.
The DesignWare DW-HDMI User Guide explains:
For Automatic CTS generation
Write "0" on the bit field "CTS_manual", Register 0x3205: AUD_CTS3
The DesignWare DW-HDMI Databook explains :
If "CTS_manual" bit equals 0b this registers contains "audCTS[19:0]"
generated by the Cycle time counter according to specified timing.
Cc: Jernej Skrabec
Cc: Maxime Ripard
Cc: Jonas Karlman
Cc: Heiko Stuebner
Cc: Jerome Brunet
Signed-off-by: Neil Armstrong
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 44 +++
1 file changed, 29 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index c68b6ed1bb35..6458c3a31d23 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -437,8 +437,14 @@ static void hdmi_set_cts_n(struct dw_hdmi *hdmi, unsigned
int cts,
/* nshift factor = 0 */
hdmi_modb(hdmi, 0, HDMI_AUD_CTS3_N_SHIFT_MASK, HDMI_AUD_CTS3);
- hdmi_writeb(hdmi, ((cts >> 16) & HDMI_AUD_CTS3_AUDCTS19_16_MASK) |
- HDMI_AUD_CTS3_CTS_MANUAL, HDMI_AUD_CTS3);
+ /* Use automatic CTS generation mode when CTS is not set */
+ if (cts)
+ hdmi_writeb(hdmi, ((cts >> 16) &
+ HDMI_AUD_CTS3_AUDCTS19_16_MASK) |
+ HDMI_AUD_CTS3_CTS_MANUAL,
+ HDMI_AUD_CTS3);
+ else
+ hdmi_writeb(hdmi, 0, HDMI_AUD_CTS3);
hdmi_writeb(hdmi, (cts >> 8) & 0xff, HDMI_AUD_CTS2);
hdmi_writeb(hdmi, cts & 0xff, HDMI_AUD_CTS1);
@@ -508,24 +514,32 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
{
unsigned long ftdms = pixel_clk;
unsigned int n, cts;
+ u8 config3;
u64 tmp;
n = hdmi_compute_n(sample_rate, pixel_clk);
- /*
-* Compute the CTS value from the N value. Note that CTS and N
-* can be up to 20 bits in total, so we need 64-bit math. Also
-* note that our TDMS clock is not fully accurate; it is accurate
-* to kHz. This can introduce an unnecessary remainder in the
-* calculation below, so we don't try to warn about that.
-*/
- tmp = (u64)ftdms * n;
- do_div(tmp, 128 * sample_rate);
- cts = tmp;
+ config3 = hdmi_readb(hdmi, HDMI_CONFIG3_ID);
- dev_dbg(hdmi->dev, "%s: fs=%uHz ftdms=%lu.%03luMHz N=%d cts=%d\n",
- __func__, sample_rate, ftdms / 100, (ftdms / 1000) % 1000,
- n, cts);
+ /* Only compute CTS when using internal AHB audio */
+ if (config3 & HDMI_CONFIG3_AHBAUDDMA) {
+ /*
+* Compute the CTS value from the N value. Note that CTS and N
+* can be up to 20 bits in total, so we need 64-bit math. Also
+* note that our TDMS clock is not fully accurate; it is
+* accurate to kHz. This can introduce an unnecessary remainder
+* in the calculation below, so we don't try to warn about that.
+*/
+ tmp = (u64)ftdms * n;
+ do_div(tmp, 128 * sample_rate);
+ cts = tmp;
+
+ dev_dbg(hdmi->dev, "%s: fs=%uHz ftdms=%lu.%03luMHz N=%d
cts=%d\n",
+ __func__, sample_rate,
+ ftdms / 100, (ftdms / 1000) % 1000,
+ n, cts);
+ } else
+ cts = 0;
spin_lock_irq(&hdmi->audio_lock);
hdmi->audio_n = n;
--
2.21.0
[Bug 110862] Dual-monitors invalid state after turning on
https://bugs.freedesktop.org/show_bug.cgi?id=110862 Michel Dänzer changed: What|Removed |Added Attachment #144506|text/x-log |text/plain mime type|| -- 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
[PATCH 2/2] drm/ast: Drop fb_debug_enter/leave
ast doesn't implement the mode_set_base_atomic hook this would need,
so this is dead code.
Signed-off-by: Daniel Vetter
Cc: Dave Airlie
Cc: Daniel Vetter
Cc: Gerd Hoffmann
Cc: Thomas Zimmermann
Cc: Alex Deucher
Cc: Sam Bobroff
Cc: Sam Ravnborg
Cc: YueHaibing
---
drivers/gpu/drm/ast/ast_fb.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
index 05f45222b702..5480caecde86 100644
--- a/drivers/gpu/drm/ast/ast_fb.c
+++ b/drivers/gpu/drm/ast/ast_fb.c
@@ -166,8 +166,6 @@ static struct fb_ops astfb_ops = {
.fb_pan_display = drm_fb_helper_pan_display,
.fb_blank = drm_fb_helper_blank,
.fb_setcmap = drm_fb_helper_setcmap,
- .fb_debug_enter = drm_fb_helper_debug_enter,
- .fb_debug_leave = drm_fb_helper_debug_leave,
};
static int astfb_create_object(struct ast_fbdev *afbdev,
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 5/7] drm/bridge: Add Analogix anx6345 support
On 04.06.2019 14:23, Torsten Duwe wrote:
> From: Icenowy Zheng
>
> The ANX6345 is an ultra-low power DisplayPower/eDP transmitter designed
> for portable devices. This driver adds initial support for RGB to eDP
> mode, without HPD and interrupts.
>
> This is a configuration usually seen in eDP applications.
>
> Signed-off-by: Icenowy Zheng
> Signed-off-by: Vasily Khoruzhick
> Signed-off-by: Torsten Duwe
> ---
> drivers/gpu/drm/bridge/analogix/Kconfig| 12 +
> drivers/gpu/drm/bridge/analogix/Makefile | 1 +
> drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 814
> +
> 3 files changed, 827 insertions(+)
> create mode 100644 drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
>
> diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig
> b/drivers/gpu/drm/bridge/analogix/Kconfig
> index 704fb0f41dc3..cd89e238b93e 100644
> --- a/drivers/gpu/drm/bridge/analogix/Kconfig
> +++ b/drivers/gpu/drm/bridge/analogix/Kconfig
> @@ -1,6 +1,18 @@
> # SPDX-License-Identifier: GPL-2.0-only
> +config DRM_ANALOGIX_ANX6345
> + tristate "Analogix ANX6345 bridge"
> + select DRM_ANALOGIX_DP
> + select DRM_KMS_HELPER
> + select REGMAP_I2C
> + help
> + ANX6345 is an ultra-low Full-HD DisplayPort/eDP
> + transmitter designed for portable devices. The
> + ANX6345 transforms the LVTTL RGB output of an
> + application processor to eDP or DisplayPort.
> +
> config DRM_ANALOGIX_ANX78XX
> tristate "Analogix ANX78XX bridge"
> + select DRM_ANALOGIX_DP
> select DRM_KMS_HELPER
> select REGMAP_I2C
> help
> diff --git a/drivers/gpu/drm/bridge/analogix/Makefile
> b/drivers/gpu/drm/bridge/analogix/Makefile
> index 7623b9b80167..97669b374098 100644
> --- a/drivers/gpu/drm/bridge/analogix/Makefile
> +++ b/drivers/gpu/drm/bridge/analogix/Makefile
> @@ -1,4 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0-only
> analogix_dp-objs := analogix_dp_core.o analogix_dp_reg.o analogix-i2c-dptx.o
> +obj-$(CONFIG_DRM_ANALOGIX_ANX6345) += analogix-anx6345.o
> obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
> obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp.o
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> new file mode 100644
> index ..ae222f9f6586
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> @@ -0,0 +1,814 @@
> +/*
> + * Copyright(c) 2016, Analogix Semiconductor.
> + * Copyright(c) 2017, Icenowy Zheng
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
SPDX
> + *
> + * Based on anx7808 driver obtained from chromeos with copyright:
> + * Copyright(c) 2013, Google Inc.
> + *
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "analogix-i2c-dptx.h"
> +#include "analogix-i2c-txcommon.h"
> +
> +#define I2C_NUM_ADDRESSES2
You can define it as ARRAY_SIZE(anx6345_i2c_addresses) - more generic.
> +#define I2C_IDX_DPTX 0
> +#define I2C_IDX_TXCOM1
> +
> +#define POLL_DELAY 5 /* us */
> +#define POLL_TIMEOUT 500 /* us */
> +
> +static const u8 anx6345_i2c_addresses[] = {
> + [I2C_IDX_DPTX] = ANALOGIX_I2C_DPTX,
> + [I2C_IDX_TXCOM] = ANALOGIX_I2C_TXCOMMON,
> +};
> +
> +struct anx6345 {
> + struct drm_dp_aux aux;
> + struct drm_bridge bridge;
> + struct i2c_client *client;
> + struct edid *edid;
> + struct drm_connector connector;
> + struct drm_dp_link link;
> + struct drm_panel *panel;
> + struct regulator *dvdd12;
> + struct regulator *dvdd25;
> + struct gpio_desc *gpiod_reset;
> + struct mutex lock; /* protect EDID access */
> +
> + /* I2C Slave addresses of ANX6345 are mapped as DPTX and SYS */
> + struct i2c_client *i2c_clients[I2C_NUM_ADDRESSES];
> + struct regmap *map[I2C_NUM_ADDRESSES];
> +
> + u16 chipid;
> + u8 dpcd[DP_RECEIVER_CAP_SIZE];
> +
> + bool powered;
> +};
> +
> +static inline struct anx6345 *connector_to_anx6345(struct drm_connector *c)
> +{
> + return container_of(c, struct anx6345, connector);
> +}
> +
> +static inline struct anx6345 *bridge_to_anx6345(struct drm_bridge *bridge)
> +{
> + return container_of(bridge, struct anx6345, bridge);
> +}
> +
> +static int anx6345_set_bits(struct regmap *map, u
[PATCH 1/2] drm/omapdrm: drop fb_debug_enter/leave
This is a no-op on atomic drivers because with atomic it's simply too
complicated to get all the locking and workers and nonblocking
synchronization correct, from essentially an NMI context. Well, too
complicated = impossible. Also, omapdrm never implemented the
mode_set_base_atomic hook, so I kinda wonder why this was ever added.
Drop the hooks.
Signed-off-by: Daniel Vetter
Cc: Tomi Valkeinen
---
drivers/gpu/drm/omapdrm/omap_fbdev.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c
b/drivers/gpu/drm/omapdrm/omap_fbdev.c
index 50aabd854f4d..0dad42e819ba 100644
--- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
+++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
@@ -87,8 +87,6 @@ static struct fb_ops omap_fb_ops = {
.fb_setcmap = drm_fb_helper_setcmap,
.fb_blank = drm_fb_helper_blank,
.fb_pan_display = omap_fbdev_pan_display,
- .fb_debug_enter = drm_fb_helper_debug_enter,
- .fb_debug_leave = drm_fb_helper_debug_leave,
.fb_ioctl = drm_fb_helper_ioctl,
.fb_read = drm_fb_helper_sys_read,
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/omapdrm: drop fb_debug_enter/leave
On 12/06/2019 12:12, Daniel Vetter wrote: This is a no-op on atomic drivers because with atomic it's simply too complicated to get all the locking and workers and nonblocking synchronization correct, from essentially an NMI context. Well, too complicated = impossible. Also, omapdrm never implemented the mode_set_base_atomic hook, so I kinda wonder why this was ever added. Drop the hooks. f9b34a0fa4e25d9c0b72f124680c37c0c38f9934 It was just open coding DRM_FB_HELPER_DEFAULT_OPS, to get rid of "Initializer entry defined twice" warning. Acked-by: Tomi Valkeinen Or I can pick it up to my branch if this is not part of a bigger series. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110862] Dual-monitors invalid state after turning on
https://bugs.freedesktop.org/show_bug.cgi?id=110862 Michel Dänzer changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |NOTOURBUG --- Comment #8 from Michel Dänzer --- The xrandr output is probably different in the bad case as well, isn't it? >From the bad case Xorg log file: [421376.800] (II) AMDGPU(0): Allocate new frame buffer 2160x738 This means Xorg calls into the driver to resize the desktop to 2160x738 (which explains why that area is OK, whereas the remaining area has undefined contents). This is most likely in turn due to a client (probably some component of the desktop environment you're using) requesting so via the RandR extension. So it's either a bug in that or in the X server, not in the driver. -- 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 109206] Kernel 4.20 amdgpu fails to load firmware on Ryzen 2500U
https://bugs.freedesktop.org/show_bug.cgi?id=109206 --- Comment #50 from Ondrej Lang --- I tested this yesterday with kernel 5.1.8 and if the file raven_dmcu.bin is present in the /lib/firmware/amdgpu/ folder when you are updating the kernel (or manually rebuilding the initramfs), the computer will boot with a blank screen next time. There are 2 pieces to this. The linux-firmware package provides the binary files (i.e. the raven_dmcu.bin) so every time this package gets updated, you should rename/move the file and rebuild the initramfs. The linux-firmware updates are not as frequent as the kernel updates so if you do the workaround, you might go through several kernel updates without issues, but once linux-firmware updates, you have to repeat the workaround... All the patch in the kernel will do is to ignore the raven_dmcu.bin file automatically (for raven 1 cpus) when building the initramfs so you don't have to rename/move it every time linux-firmware updates. -- 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 7/7] arm64: dts: allwinner: a64: enable ANX6345 bridge on Teres-I
On 07.06.2019 11:40, Torsten Duwe wrote: > On Fri, Jun 07, 2019 at 08:28:02AM +0200, Maxime Ripard wrote: >> On Thu, Jun 06, 2019 at 03:59:27PM +0200, Harald Geyer wrote: >>> If think valid compatible properties would be: >>> compatible = "innolux,n116bge", "simple-panel"; >>> compatible = "edp-connector", "simple-panel"; >> A connector isn't a panel. >> >>> compatible = "innolux,n116bge", "edp-connector", "simple-panel"; >> And the innolux,n116bge is certainly not a connector either. >> >>> compatible = "edp-connector", "innolux,n116bge", "simple-panel"; >>> >>> I can't make up my mind which one I prefere. However neither of these >>> variants requires actually implmenting an edp-connector driver. >> No-one asked to do an edp-connector driver. You should use it in your >> DT, but if you want to have some code in your driver that parses the >> DT directly, I'm totally fine with that. > I must admit I fail to understand what that extra node would be good for. > Logically, the eDP far side is connected to the well-known n116bge. > Inside the laptop case it might as well be a flat ribbon cable or > soldered directly. > In good intention, that's all I wanted to express in the DT. I don't > know whether the relevant mechanical dimensions of the panel and the > connector are standardised, so whether one could in theory assemble it > with a different panel than the one it came with. > > OTOH, as I checked during the discussion with anarsoul, the panel's > supply voltage is permanently connected to the main 3.3V rail. > We already agreed that the eDP output port must not neccessarily be > specified, this setup is a good example why: because the panel is > always powered, the anx6345 can always pull valid EDID data from it > so at this stage there's no need for any OS driver to reach beyond > the bridge. IIRC even the backlight got switched off for the blank > screen without. > > All I wanted to say is that "there's usually an n116bge behind it"; > but this is mostly redundant. > > So, shall we just drop the output port specification (along with the > panel node) in order to get one step further? I am not sure if I understand whole discussion here, but I also do not understand whole edp-connector thing. According to VESA[1] eDP is "Internal display interface" - there is no external connector for eDP, the way it is connected is integrator's decision, but it is fixed - ie end user do not plug/unplug it. If I remember correctly in some boards eDP is connected to some DP connector (odroid xu3 if I remember correctly), but this is non-standard hack, and for this case in bindings there should be rather dp-connector not edp-connector. [1]: https://www.vesa.org/wp-content/uploads/2010/12/DisplayPort-DevCon-Presentation-eDP-Dec-2010-v3.pdf Regards Andrzej > >> I guess you should describe why do you think it's "clear", because I'm >> not sure this is obvious for everyone here. eDP allows to discover >> which device is on the other side and its supported timings, just like >> HDMI for example (or regular DP, for that matter). Would you think >> it's clearly preferable to ship a DT with the DP/HDMI monitor >> connected on the other side exposed as a panel as well? > Well, as I wrote: "in good intention". That's the panel that comes with > the kit but it is very well detected automatically, just like you describe. > > So, just leave it out? > > Torsten > > >
Re: [PATCH v2 1/7] drm/bridge: move ANA78xx driver to analogix subdirectory
On 04.06.2019 14:22, Torsten Duwe wrote:
> From: Icenowy Zheng
>
> As ANA78xx chips are designed and produced by Analogix Semiconductor,
> Inc, move their driver codes into analogix subdirectory.
>
> Signed-off-by: Icenowy Zheng
> Signed-off-by: Vasily Khoruzhick
> Reviewed-by: Laurent Pinchart
> Signed-off-by: Torsten Duwe
Reviewed-by: Andrzej Hajda
--
Regards
Andrzej
> ---
> drivers/gpu/drm/bridge/Kconfig | 10 --
> drivers/gpu/drm/bridge/Makefile | 4 ++--
> drivers/gpu/drm/bridge/analogix/Kconfig | 10 ++
> drivers/gpu/drm/bridge/analogix/Makefile | 1 +
> drivers/gpu/drm/bridge/{ => analogix}/analogix-anx78xx.c | 0
> drivers/gpu/drm/bridge/{ => analogix}/analogix-anx78xx.h | 0
> 6 files changed, 13 insertions(+), 12 deletions(-)
> rename drivers/gpu/drm/bridge/{ => analogix}/analogix-anx78xx.c (100%)
> rename drivers/gpu/drm/bridge/{ => analogix}/analogix-anx78xx.h (100%)
>
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index ee777469293a..862789bf64a0 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -16,16 +16,6 @@ config DRM_PANEL_BRIDGE
> menu "Display Interface Bridges"
> depends on DRM && DRM_BRIDGE
>
> -config DRM_ANALOGIX_ANX78XX
> - tristate "Analogix ANX78XX bridge"
> - select DRM_KMS_HELPER
> - select REGMAP_I2C
> - ---help---
> - ANX78XX is an ultra-low Full-HD SlimPort transmitter
> - designed for portable devices. The ANX78XX transforms
> - the HDMI output of an application processor to MyDP
> - or DisplayPort.
> -
> config DRM_CDNS_DSI
> tristate "Cadence DPI/DSI bridge"
> select DRM_KMS_HELPER
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 4934fcf5a6f8..a6c7dd7727ea 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -1,5 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0
> -obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
> obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o
> obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o
> obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o
> @@ -12,8 +11,9 @@ obj-$(CONFIG_DRM_SII9234) += sii9234.o
> obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o
> obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o
> obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
> -obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
> obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
> obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
> obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
> +
> +obj-y += analogix/
> obj-y += synopsys/
> diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig
> b/drivers/gpu/drm/bridge/analogix/Kconfig
> index e930ff9b5cd4..704fb0f41dc3 100644
> --- a/drivers/gpu/drm/bridge/analogix/Kconfig
> +++ b/drivers/gpu/drm/bridge/analogix/Kconfig
> @@ -1,4 +1,14 @@
> # SPDX-License-Identifier: GPL-2.0-only
> +config DRM_ANALOGIX_ANX78XX
> + tristate "Analogix ANX78XX bridge"
> + select DRM_KMS_HELPER
> + select REGMAP_I2C
> + help
> + ANX78XX is an ultra-low Full-HD SlimPort transmitter
> + designed for portable devices. The ANX78XX transforms
> + the HDMI output of an application processor to MyDP
> + or DisplayPort.
> +
> config DRM_ANALOGIX_DP
> tristate
> depends on DRM
> diff --git a/drivers/gpu/drm/bridge/analogix/Makefile
> b/drivers/gpu/drm/bridge/analogix/Makefile
> index fdbf3fd2f087..6fcbfd3ee560 100644
> --- a/drivers/gpu/drm/bridge/analogix/Makefile
> +++ b/drivers/gpu/drm/bridge/analogix/Makefile
> @@ -1,3 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0-only
> analogix_dp-objs := analogix_dp_core.o analogix_dp_reg.o
> +obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
> obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp.o
> diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c
> b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> similarity index 100%
> rename from drivers/gpu/drm/bridge/analogix-anx78xx.c
> rename to drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.h
> b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.h
> similarity index 100%
> rename from drivers/gpu/drm/bridge/analogix-anx78xx.h
> rename to drivers/gpu/drm/bridge/analogix/analogix-anx78xx.h
[PATCH v2 0/2] drm/komeda: Add writeback downscaling split support
Writeback split is also for workaround the size limitation of d71 scaler. Like layer_split, writeback downscaling also can use two scalers to handle the scaling half-by-half. The only differnence is writback needs a standalone component (splitter)'s help to split the composition result. The data pipeline of writeback split as below: /-> scaler-0 ->\ compiz -> splittermerger -> wb_layer -> memory \-> scaler-1 ->/ Depends on: - https://patchwork.freedesktop.org/series/58710/ - https://patchwork.freedesktop.org/series/59000/ - https://patchwork.freedesktop.org/series/59002/ - https://patchwork.freedesktop.org/series/59747/ - https://patchwork.freedesktop.org/series/59915/ - https://patchwork.freedesktop.org/series/60083/ - https://patchwork.freedesktop.org/series/60698/ - https://patchwork.freedesktop.org/series/60856/ - https://patchwork.freedesktop.org/series/61079/ - https://patchwork.freedesktop.org/series/61081/ v2: Rebase james qian wang (Arm Technology China) (2): drm/komeda: Add new component komeda_splitter drm/komeda: Enable writeback split support .../arm/display/komeda/d71/d71_component.c| 63 ++ .../drm/arm/display/komeda/komeda_pipeline.c | 3 + .../drm/arm/display/komeda/komeda_pipeline.h | 23 +++- .../display/komeda/komeda_pipeline_state.c| 117 -- .../arm/display/komeda/komeda_private_obj.c | 50 .../arm/display/komeda/komeda_wb_connector.c | 17 ++- 6 files changed, 257 insertions(+), 16 deletions(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/2] drm/komeda: Add new component komeda_splitter
Similar to Layer Split, but Splitter is used for writeback, which splits
the compiz result to two half parts and then feed them to two scalers.
v2: Rebase
Signed-off-by: James Qian Wang (Arm Technology China)
---
.../arm/display/komeda/d71/d71_component.c| 63 +++
.../drm/arm/display/komeda/komeda_pipeline.c | 3 +
.../drm/arm/display/komeda/komeda_pipeline.h | 19 +-
.../arm/display/komeda/komeda_private_obj.c | 50 +++
4 files changed, 133 insertions(+), 2 deletions(-)
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 f40fdd175479..4b19a9aefa90 100644
--- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
+++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
@@ -808,6 +808,68 @@ static int d71_downscaling_clk_check(struct
komeda_pipeline *pipe,
0 : -EINVAL;
}
+static void d71_splitter_update(struct komeda_component *c,
+ struct komeda_component_state *state)
+{
+ struct komeda_splitter_state *st = to_splitter_st(state);
+ u32 __iomem *reg = c->reg;
+
+ malidp_write32(reg, BLK_INPUT_ID0, to_d71_input_id(state, 0));
+ malidp_write32(reg, BLK_SIZE, HV_SIZE(st->hsize, st->vsize));
+ malidp_write32(reg, SP_OVERLAP_SIZE, st->overlap & 0x1FFF);
+ malidp_write32(reg, BLK_CONTROL, BLK_CTRL_EN);
+}
+
+static void d71_splitter_dump(struct komeda_component *c, struct seq_file *sf)
+{
+ u32 v[3];
+
+ dump_block_header(sf, c->reg);
+
+ get_values_from_reg(c->reg, BLK_INPUT_ID0, 1, v);
+ seq_printf(sf, "SP_INPUT_ID0:\t\t0x%X\n", v[0]);
+
+ get_values_from_reg(c->reg, BLK_CONTROL, 3, v);
+ seq_printf(sf, "SP_CONTROL:\t\t0x%X\n", v[0]);
+ seq_printf(sf, "SP_SIZE:\t\t0x%X\n", v[1]);
+ seq_printf(sf, "SP_OVERLAP_SIZE:\t0x%X\n", v[2]);
+}
+
+static const struct komeda_component_funcs d71_splitter_funcs = {
+ .update = d71_splitter_update,
+ .disable= d71_component_disable,
+ .dump_register = d71_splitter_dump,
+};
+
+static int d71_splitter_init(struct d71_dev *d71,
+struct block_header *blk, u32 __iomem *reg)
+{
+ struct komeda_component *c;
+ struct komeda_splitter *splitter;
+ u32 pipe_id, comp_id;
+
+ get_resources_id(blk->block_info, &pipe_id, &comp_id);
+
+ c = komeda_component_add(&d71->pipes[pipe_id]->base, sizeof(*splitter),
+comp_id,
+BLOCK_INFO_INPUT_ID(blk->block_info),
+&d71_splitter_funcs,
+1, get_valid_inputs(blk), 2, reg,
+"CU%d_SPLITTER", pipe_id);
+
+ if (IS_ERR(c)) {
+ DRM_ERROR("Failed to initialize splitter");
+ return -1;
+ }
+
+ splitter = to_splitter(c);
+
+ set_range(&splitter->hsize, 4, d71->max_line_size);
+ set_range(&splitter->vsize, 4, d71->max_vsize);
+
+ return 0;
+}
+
static void d71_merger_update(struct komeda_component *c,
struct komeda_component_state *state)
{
@@ -1102,6 +1164,7 @@ int d71_probe_block(struct d71_dev *d71,
break;
case D71_BLK_TYPE_CU_SPLITTER:
+ err = d71_splitter_init(d71, blk, reg);
break;
case D71_BLK_TYPE_CU_MERGER:
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
index eb9e0c0af8f3..c0130f1fac44 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
@@ -91,6 +91,9 @@ komeda_pipeline_get_component_pos(struct komeda_pipeline
*pipe, int id)
case KOMEDA_COMPONENT_SCALER1:
pos = to_cpos(pipe->scalers[id - KOMEDA_COMPONENT_SCALER0]);
break;
+ case KOMEDA_COMPONENT_SPLITTER:
+ pos = to_cpos(pipe->splitter);
+ break;
case KOMEDA_COMPONENT_MERGER:
pos = to_cpos(pipe->merger);
break;
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
index f6a4a51cb5f7..b6fed54b1cf1 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
@@ -301,6 +301,17 @@ struct komeda_merger_state {
u16 vsize_merged;
};
+struct komeda_splitter {
+ struct komeda_component base;
+ struct malidp_range hsize, vsize;
+};
+
+struct komeda_splitter_state {
+ struct komeda_component_state base;
+ u16 hsize, vsize;
+ u16 overlap;
+};
+
struct komeda_improc {
struct komeda_component base;
u32 supported_color_formats; /* DRM_RGB/YUV444/YUV420*/
@@ -388,6 +399,8 @@ struct komeda_pipeline {
[PATCH v2 2/2] drm/komeda: Enable writeback split support
Writeback split is also for workaround the size limitation of d71 scaler.
Like layer_split, writeback downscaling also can use two scalers to handle
the scaling half-by-half. The only differnence is writback needs a
standalone component (splitter)'s help to split the composition result.
The data pipeline of writeback split as below:
/-> scaler-0 ->\
compiz -> splittermerger -> wb_layer -> memory
\-> scaler-1 ->/
v2: Rebase
Signed-off-by: James Qian Wang (Arm Technology China)
---
.../drm/arm/display/komeda/komeda_pipeline.h | 4 +
.../display/komeda/komeda_pipeline_state.c| 117 --
.../arm/display/komeda/komeda_wb_connector.c | 17 ++-
3 files changed, 124 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
index b6fed54b1cf1..c0378b18f803 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
@@ -519,6 +519,10 @@ int komeda_build_layer_split_data_flow(struct komeda_layer
*left,
struct komeda_plane_state *kplane_st,
struct komeda_crtc_state *kcrtc_st,
struct komeda_data_flow_cfg *dflow);
+int komeda_build_wb_split_data_flow(struct komeda_layer *wb_layer,
+ struct drm_connector_state *conn_st,
+ struct komeda_crtc_state *kcrtc_st,
+ struct komeda_data_flow_cfg *dflow);
int komeda_release_unclaimed_resources(struct komeda_pipeline *pipe,
struct komeda_crtc_state *kcrtc_st);
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
index 23182edeb09a..b58a32f1b158 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
@@ -284,30 +284,33 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
struct komeda_fb *kfb,
struct komeda_data_flow_cfg *dflow)
{
- u32 hsize_in, vsize_in;
+ u32 src_x, src_y, src_w, src_h;
if (!komeda_fb_is_layer_supported(kfb, layer->layer_type, dflow->rot))
return -EINVAL;
- if (komeda_fb_check_src_coords(kfb, dflow->in_x, dflow->in_y,
- dflow->in_w, dflow->in_h))
- return -EINVAL;
-
if (layer->base.id == KOMEDA_COMPONENT_WB_LAYER) {
- hsize_in = dflow->out_w;
- vsize_in = dflow->out_h;
+ src_x = dflow->out_x;
+ src_y = dflow->out_y;
+ src_w = dflow->out_w;
+ src_h = dflow->out_h;
} else {
- hsize_in = dflow->in_w;
- vsize_in = dflow->in_h;
+ src_x = dflow->in_x;
+ src_y = dflow->in_y;
+ src_w = dflow->in_w;
+ src_h = dflow->in_h;
}
- if (!in_range(&layer->hsize_in, hsize_in)) {
- DRM_DEBUG_ATOMIC("invalidate src_w %d.\n", hsize_in);
+ if (komeda_fb_check_src_coords(kfb, src_x, src_y, src_w, src_h))
+ return -EINVAL;
+
+ if (!in_range(&layer->hsize_in, src_w)) {
+ DRM_DEBUG_ATOMIC("invalidate src_w %d.\n", src_w);
return -EINVAL;
}
- if (!in_range(&layer->vsize_in, vsize_in)) {
- DRM_DEBUG_ATOMIC("invalidate src_h %d.\n", vsize_in);
+ if (!in_range(&layer->vsize_in, src_h)) {
+ DRM_DEBUG_ATOMIC("invalidate src_h %d.\n", src_h);
return -EINVAL;
}
@@ -534,6 +537,59 @@ komeda_scaler_validate(void *user,
return err;
}
+static void komeda_split_data_flow(struct komeda_scaler *scaler,
+ struct komeda_data_flow_cfg *dflow,
+ struct komeda_data_flow_cfg *l_dflow,
+ struct komeda_data_flow_cfg *r_dflow);
+
+static int
+komeda_splitter_validate(struct komeda_splitter *splitter,
+struct drm_connector_state *conn_st,
+struct komeda_data_flow_cfg *dflow,
+struct komeda_data_flow_cfg *l_output,
+struct komeda_data_flow_cfg *r_output)
+{
+ struct komeda_component_state *c_st;
+ struct komeda_splitter_state *st;
+
+ if (!splitter) {
+ DRM_DEBUG_ATOMIC("Current HW doesn't support splitter.\n");
+ return -EINVAL;
+ }
+
+ if (!in_range(&splitter->hsize, dflow->in_w)) {
+ DRM_DEBUG_ATOMIC("split in_w:%d is out of the acceptable
range.\n",
+dflow->in_w);
+
Re: [PATCH v3 08/10] arm64: dts: renesas: r8a7799[05]: Point LVDS0 to its companion LVDS1
Hi Simon, On Thu, Jun 06, 2019 at 10:51:09AM +0200, Simon Horman wrote: > On Thu, Jun 06, 2019 at 10:59:57AM +0300, Laurent Pinchart wrote: > > On Mon, Jun 03, 2019 at 01:40:45PM +0200, Simon Horman wrote: > >> On Tue, May 28, 2019 at 05:12:32PM +0300, Laurent Pinchart wrote: > >>> Add the new renesas,companion property to the LVDS0 node to point to the > >>> companion LVDS encoder LVDS1. > >>> > >>> Signed-off-by: Laurent Pinchart > >>> > >>> Reviewed-by: Jacopo Mondi > >>> Tested-by: Jacopo Mondi > >> > >> Hi Laurent, > >> > >> is this patch ready to be merged? > > > > I was hoping for an ack from the DT bindings maintainers for the > > corresponding bindings changes, but I want to get this merged for the > > next kernel release, so I may not get it. I'll ping you when I send the > > pull request for the DT bindings and drivers changes, at that point this > > patch should be picked up. > > Thanks Laurent, got it. The DT bindings and driver changes have been merged, so please go ahead and pick this patch for v5.3. -- Regards, Laurent Pinchart
Re: [PATCH] video: backlight: Replace old GPIO APIs with GPIO Consumer APIs for sky81542-backlight driver
Hi Shobhit
Thanks for the patch. Feedback below...
On Tue, Jun 11, 2019 at 09:32:32PM -0700, Shobhit Kukreti wrote:
> Port the sky81452-backlight driver to adhere to new gpio descriptor based
> APIs. Modified the file sky81452-backlight.c and sky81452-backlight.h.
> The gpio descriptor property in device tree should be "sky81452-en-gpios"
That is contradicted by the device tree bindings. The property should
remain "gpios" as it was before this conversion.
> Removed unnecessary header files "linux/gpio.h" and "linux/of_gpio.h".
>
> Signed-off-by: Shobhit Kukreti
What level of testing have you done? Is this a fix for hardware you own
or a cleanup after searching the sources?
> ---
> drivers/video/backlight/sky81452-backlight.c | 24
>
> include/linux/platform_data/sky81452-backlight.h | 4 +++-
> 2 files changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/video/backlight/sky81452-backlight.c
> b/drivers/video/backlight/sky81452-backlight.c
> index d414c7a..12ef628 100644
> --- a/drivers/video/backlight/sky81452-backlight.c
> +++ b/drivers/video/backlight/sky81452-backlight.c
> @@ -19,12 +19,10 @@
>
> #include
> #include
> -#include
> #include
> #include
> #include
> #include
> -#include
> #include
> #include
> #include
> @@ -193,7 +191,6 @@ static struct sky81452_bl_platform_data
> *sky81452_bl_parse_dt(
> pdata->ignore_pwm = of_property_read_bool(np, "skyworks,ignore-pwm");
> pdata->dpwm_mode = of_property_read_bool(np, "skyworks,dpwm-mode");
> pdata->phase_shift = of_property_read_bool(np, "skyworks,phase-shift");
> - pdata->gpio_enable = of_get_gpio(np, 0);
>
> ret = of_property_count_u32_elems(np, "led-sources");
> if (ret < 0) {
> @@ -274,13 +271,17 @@ static int sky81452_bl_probe(struct platform_device
> *pdev)
> if (IS_ERR(pdata))
> return PTR_ERR(pdata);
> }
> -
> - if (gpio_is_valid(pdata->gpio_enable)) {
> - ret = devm_gpio_request_one(dev, pdata->gpio_enable,
> - GPIOF_OUT_INIT_HIGH, "sky81452-en");
> - if (ret < 0) {
> - dev_err(dev, "failed to request GPIO. err=%d\n", ret);
> - return ret;
> + pdata->gpiod_enable = devm_gpiod_get(dev, "sk81452-en", GPIOD_OUT_HIGH);
As above... I think the second argument here needs to be NULL in order
to preserve the current DT bindings.
> + if (IS_ERR(pdata->gpiod_enable)) {
> + long ret = PTR_ERR(pdata->gpiod_enable);
> +
> + /**
Nitpicking... but no second star here. That's a trigger symbold for
documentation processors.
> + * gpiod_enable is optional in device tree.
> + * Return error only if gpio was assigned in device tree
Also nitpicking but I had to read this a few times because
gpiod_enable is not in device tree, gpios is.
This is a common pattern so the comment can be very short. Something
like:
This DT property is optional so no need to propagate ENOENT
> + */
> + if (ret != -ENOENT) {
> + dev_err(dev, "failed to request GPIO. err=%ld\n", ret);
> + return PTR_ERR(pdata->gpiod_enable);
> }
> }
>
> @@ -323,8 +324,7 @@ static int sky81452_bl_remove(struct platform_device
> *pdev)
> bd->props.brightness = 0;
> backlight_update_status(bd);
>
> - if (gpio_is_valid(pdata->gpio_enable))
> - gpio_set_value_cansleep(pdata->gpio_enable, 0);
> + gpiod_set_value_cansleep(pdata->gpiod_enable, 0);
>
> return 0;
> }
> diff --git a/include/linux/platform_data/sky81452-backlight.h
> b/include/linux/platform_data/sky81452-backlight.h
> index 1231e9b..dc4cb85 100644
> --- a/include/linux/platform_data/sky81452-backlight.h
> +++ b/include/linux/platform_data/sky81452-backlight.h
> @@ -20,6 +20,8 @@
> #ifndef _SKY81452_BACKLIGHT_H
> #define _SKY81452_BACKLIGHT_H
>
> +#include
> +
This heaer file should be included from the C file... it is not required
to parse the header.
Daniel.
> /**
> * struct sky81452_platform_data
> * @name:backlight driver name.
> @@ -34,7 +36,7 @@
> */
> struct sky81452_bl_platform_data {
> const char *name;
> - int gpio_enable;
> + struct gpio_desc *gpiod_enable;
> unsigned int enable;
> bool ignore_pwm;
> bool dpwm_mode;
> --
> 2.7.4
>
Re: [PATCH v5 00/15] tc358767 driver improvements
On 12.06.2019 10:32, Andrey Smirnov wrote: > Everyone: > > This series contains various improvements (at least in my mind) and > fixes that I made to tc358767 while working with the code of the > driver. Hopefuly each patch is self explanatory. > > Feedback is welcome! > > Thanks, > Andrey Smirnov Tomi, you can queue it to drm-misc-next. Regards Andrzej > > Changes since [v4]: > > - tc_pllupdate_pllen() renamed to tc_pllupdate() > > - Collected Reviewed-bys from Andrzej for the rest of the series > > Changes since [v3]: > > - Collected Reviewed-bys from Andrzej > > - Dropped explicit check for -ETIMEDOUT in "drm/bridge: tc358767: > Simplify polling in tc_main_link_setup()" for consistency > > - AUX transfer code converted to user regmap_raw_read(), > regmap_raw_write() > > Changes since [v2]: > > - Patchset rebased on top of v4 of Tomi's series that recently > went in (https://patchwork.freedesktop.org/series/58176/#rev5) > > - AUX transfer code converted to user regmap_bulk_read(), > regmap_bulk_write() > > Changes since [v1]: > > - Patchset rebased on top of > https://patchwork.freedesktop.org/series/58176/ > > - Patches to remove both tc_write() and tc_read() helpers added > > - Patches to rework AUX transfer code added > > - Both "drm/bridge: tc358767: Simplify polling in > tc_main_link_setup()" and "drm/bridge: tc358767: Simplify > polling in tc_link_training()" changed to use tc_poll_timeout() > instead of regmap_read_poll_timeout() > > [v4] lkml.kernel.org/r/20190607044550.13361-1-andrew.smir...@gmail.com > [v3] lkml.kernel.org/r/20190605070507.11417-1-andrew.smir...@gmail.com > [v2] lkml.kernel.org/r/20190322032901.12045-1-andrew.smir...@gmail.com > [v1] lkml.kernel.org/r/20190226193609.9862-1-andrew.smir...@gmail.com > > Andrey Smirnov (15): > drm/bridge: tc358767: Simplify tc_poll_timeout() > drm/bridge: tc358767: Simplify polling in tc_main_link_setup() > drm/bridge: tc358767: Simplify polling in tc_link_training() > drm/bridge: tc358767: Simplify tc_set_video_mode() > drm/bridge: tc358767: Drop custom tc_write()/tc_read() accessors > drm/bridge: tc358767: Simplify AUX data read > drm/bridge: tc358767: Simplify AUX data write > drm/bridge: tc358767: Increase AUX transfer length limit > drm/bridge: tc358767: Use reported AUX transfer size > drm/bridge: tc358767: Add support for address-only I2C transfers > drm/bridge: tc358767: Introduce tc_set_syspllparam() > drm/bridge: tc358767: Introduce tc_pllupdate() > drm/bridge: tc358767: Simplify tc_aux_wait_busy() > drm/bridge: tc358767: Drop unnecessary 8 byte buffer > drm/bridge: tc358767: Replace magic number in tc_main_link_enable() > > drivers/gpu/drm/bridge/tc358767.c | 648 +- > 1 file changed, 372 insertions(+), 276 deletions(-) > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm: add fallback override/firmware EDID modes workaround
On Tue, 11 Jun 2019, Paul Wise wrote:
> On Mon, 2019-06-10 at 12:30 +0300, Jani Nikula wrote:
>> We've moved the override and firmware EDID (simply "override EDID" from
>> now on) handling to the low level drm_do_get_edid() function in order to
>> transparently use the override throughout the stack. The idea is that
>> you get the override EDID via the ->get_modes() hook.
>>
>> Unfortunately, there are scenarios where the DDC probe in drm_get_edid()
>> called via ->get_modes() fails, although the preceding ->detect()
>> succeeds.
>>
>> In the case reported by Paul Wise, the ->detect() hook,
>> intel_crt_detect(), relies on hotplug detect, bypassing the DDC. In the
>> case reported by Ilpo Järvinen, there is no ->detect() hook, which is
>> interpreted as connected. The subsequent DDC probe reached via
>> ->get_modes() fails, and we don't even look at the override EDID,
>> resulting in no modes being added.
>>
>> Because drm_get_edid() is used via ->detect() all over the place, we
>> can't trivially remove the DDC probe, as it leads to override EDID
>> effectively meaning connector forcing. The goal is that connector
>> forcing and override EDID remain orthogonal.
>>
>> Generally, the underlying problem here is the conflation of ->detect()
>> and ->get_modes() via drm_get_edid(). The former should just detect, and
>> the latter should just get the modes, typically via reading the EDID. As
>> long as drm_get_edid() is used in ->detect(), it needs to retain the DDC
>> probe. Or such users need to have a separate DDC probe step first.
>>
>> The EDID caching between ->detect() and ->get_modes() done by some
>> drivers is a further complication that prevents us from making
>> drm_do_get_edid() adapt to the two cases.
>>
>> Work around the regression by falling back to a separate attempt at
>> getting the override EDID at drm_helper_probe_single_connector_modes()
>> level. With a working DDC and override EDID, it'll never be called; the
>> override EDID will come via ->get_modes(). There will still be a failing
>> DDC probe attempt in the cases that require the fallback.
>>
>> v2:
>> - Call drm_connector_update_edid_property (Paul)
>> - Update commit message about EDID caching (Daniel)
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107583
>> Reported-by: Paul Wise
>> Cc: Paul Wise
>> References:
>> alpine.DEB.2.20.1905262211270.24390@whs-18.cs.helsinki.fi">http://mid.mail-archive.com/alpine.DEB.2.20.1905262211270.24390@whs-18.cs.helsinki.fi
>> Reported-by: Ilpo Järvinen
>> Cc: Ilpo Järvinen
>> Suggested-by: Daniel Vetter
>> References: 15f080f08d48 ("drm/edid: respect connector force for
>> drm_get_edid ddc probe")
>> Fixes: 53fd40a90f3c ("drm: handle override and firmware EDID at
>> drm_do_get_edid() level")
>> Cc: # v4.15+
>> Cc: Daniel Vetter
>> Cc: Ville Syrjälä
>> Cc: Harish Chegondi
>> Signed-off-by: Jani Nikula
>> ---
>> drivers/gpu/drm/drm_edid.c | 30 ++
>> drivers/gpu/drm/drm_probe_helper.c | 7 +++
>> include/drm/drm_edid.h | 1 +
>> 3 files changed, 38 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index c59a1e8c5ada..9d8f2b952004 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -1587,6 +1587,36 @@ static struct edid *drm_get_override_edid(struct
>> drm_connector *connector)
>> return IS_ERR(override) ? NULL : override;
>> }
>>
>> +/**
>> + * drm_add_override_edid_modes - add modes from override/firmware EDID
>> + * @connector: connector we're probing
>> + *
>> + * Add modes from the override/firmware EDID, if available. Only to be used
>> from
>> + * drm_helper_probe_single_connector_modes() as a fallback for when DDC
>> probe
>> + * failed during drm_get_edid() and caused the override/firmware EDID to be
>> + * skipped.
>> + *
>> + * Return: The number of modes added or 0 if we couldn't find any.
>> + */
>> +int drm_add_override_edid_modes(struct drm_connector *connector)
>> +{
>> +struct edid *override;
>> +int num_modes = 0;
>> +
>> +override = drm_get_override_edid(connector);
>> +if (override) {
>> +drm_connector_update_edid_property(connector, override);
>> +num_modes = drm_add_edid_modes(connector, override);
>> +kfree(override);
>> +
>> +DRM_DEBUG_KMS("[CONNECTOR:%d:%s] adding %d modes via fallback
>> override/firmware EDID\n",
>> + connector->base.id, connector->name, num_modes);
>> +}
>> +
>> +return num_modes;
>> +}
>> +EXPORT_SYMBOL(drm_add_override_edid_modes);
>> +
>> /**
>> * drm_do_get_edid - get EDID data using a custom EDID block read function
>> * @connector: connector we're probing
>> diff --git a/drivers/gpu/drm/drm_probe_helper.c
>> b/drivers/gpu/drm/drm_probe_helper.c
>> index 01e243f1ea94..ef2c468205a2 100644
>> --- a/drivers/gpu/drm/drm_probe_helper.c
>> +++ b/drivers/gpu/drm/drm_probe_helper.c
Re: [v2,1/2] drm/komeda: Add slave pipeline support
On Tue, Jun 11, 2019 at 11:13:39AM +, Lowry Li (Arm Technology China) wrote:
> From: "Lowry Li (Arm Technology China)"
>
> One crtc can use two komeda_pipeline, and one works as master and as
> slave. the slave pipeline doesn't have its own output and timing
> ctrlr, but pre-composite the input layer data flow and then feed the
> result to master. the pipeline configuration like:
>
> slave-layer-0 \
> ...slave->CU
> slave-layer-4 / \
> \
> master-layer-0 > master->CU -> ...
> ... /
> master-layer-4 -->
>
> Since komeda Compiz doesn't output alpha, so the slave->CU result
> only can be used as bottom input when blend it with master input data
> flows.
>
> Signed-off-by: Lowry Li (Arm Technology China)
> ---
> drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 41
> --
> drivers/gpu/drm/arm/display/komeda/komeda_kms.c| 10 ++
> drivers/gpu/drm/arm/display/komeda/komeda_kms.h| 9 +
> .../gpu/drm/arm/display/komeda/komeda_pipeline.c | 22
> .../gpu/drm/arm/display/komeda/komeda_pipeline.h | 2 ++
> .../drm/arm/display/komeda/komeda_pipeline_state.c | 15
> drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 32 -
> 7 files changed, 128 insertions(+), 3 deletions(-)
>
> --
> 1.9.1
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> index a2d656f..cafb445 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> @@ -64,6 +64,10 @@ static void komeda_crtc_update_clock_ratio(struct
> komeda_crtc_state *kcrtc_st)
> }
>
> /* release unclaimed pipeline resources */
> + err = komeda_release_unclaimed_resources(kcrtc->slave, kcrtc_st);
> + if (err)
> + return err;
> +
> err = komeda_release_unclaimed_resources(kcrtc->master, kcrtc_st);
> if (err)
> return err;
> @@ -226,6 +230,7 @@ void komeda_crtc_handle_event(struct komeda_crtc *kcrtc,
> struct komeda_crtc_state *kcrtc_st = to_kcrtc_st(crtc->state);
> struct komeda_dev *mdev = kcrtc->base.dev->dev_private;
> struct komeda_pipeline *master = kcrtc->master;
> + struct komeda_pipeline *slave = kcrtc->slave;
> struct komeda_wb_connector *wb_conn = kcrtc->wb_conn;
> struct drm_connector_state *conn_st;
>
> @@ -237,6 +242,9 @@ void komeda_crtc_handle_event(struct komeda_crtc *kcrtc,
> if (has_bit(master->id, kcrtc_st->affected_pipes))
> komeda_pipeline_update(master, old->state);
>
> + if (slave && has_bit(slave->id, kcrtc_st->affected_pipes))
> + komeda_pipeline_update(slave, old->state);
> +
> conn_st = wb_conn ? wb_conn->base.base.state : NULL;
> if (conn_st && conn_st->writeback_job)
> drm_writeback_queue_job(&wb_conn->base, conn_st);
> @@ -262,6 +270,7 @@ void komeda_crtc_handle_event(struct komeda_crtc *kcrtc,
> struct komeda_crtc_state *old_st = to_kcrtc_st(old);
> struct komeda_dev *mdev = crtc->dev->dev_private;
> struct komeda_pipeline *master = kcrtc->master;
> + struct komeda_pipeline *slave = kcrtc->slave;
> struct completion *disable_done = &crtc->state->commit->flip_done;
> struct completion temp;
> int timeout;
> @@ -270,6 +279,9 @@ void komeda_crtc_handle_event(struct komeda_crtc *kcrtc,
>drm_crtc_index(crtc),
>old_st->active_pipes, old_st->affected_pipes);
>
> + if (slave && has_bit(slave->id, old_st->active_pipes))
> + komeda_pipeline_disable(slave, old->state);
> +
> if (has_bit(master->id, old_st->active_pipes))
> komeda_pipeline_disable(master, old->state);
>
> @@ -414,6 +426,7 @@ static void komeda_crtc_reset(struct drm_crtc *crtc)
>
> new->affected_pipes = old->active_pipes;
> new->clock_ratio = old->clock_ratio;
> + new->max_slave_zorder = old->max_slave_zorder;
>
> return &new->base;
> }
> @@ -488,7 +501,7 @@ int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms,
> master = mdev->pipelines[i];
>
> crtc->master = master;
> - crtc->slave = NULL;
> + crtc->slave = komeda_pipeline_get_slave(master);
>
> if (crtc->slave)
> sprintf(str, "pipe-%d", crtc->slave->id);
> @@ -522,6 +535,26 @@ static int
> komeda_crtc_create_clock_ratio_property(struct komeda_crtc *kcrtc)
> return 0;
> }
>
> +static int komeda_crtc_create_slave_planes_property(struct komeda_crtc
> *kcrtc)
> +{
> + struct drm_crtc *crtc = &kcrtc->base;
> + struct drm_property *prop;
> +
> + if (kcrtc->slave_planes == 0)
> + return 0;
> +
> + prop = drm_property_create_range(crtc->dev, DRM_MODE_PROP_IMMUTABLE,
> +
Re: DRM/AST regression (likely 4.14 -> 4.19+), providing EDID manually fails
On Sun, 26 May 2019, Ilpo Järvinen wrote: > Hi all, > > I've a workstation which has internal VGA that is detected as AST 2400 and > with it EDID has been always quite flaky (except for some time it worked > with 4.14 long enough that I thought the problems would be past until the > problems reappeared also with 4.14). Thus, I've provided manually the EDID > that I extracted from the monitor using other computer (the monitor itself > worked just fine on the earlier computer so it is likely fine). > > I setup the manual EDID using drm_kms_helper.edid_firmware, however, > after upgrading to 4.19.45 it stopped working (no "Got external EDID base > block" appears in dmesg, the text mode is kept in the lower res mode, and > Xorg logs no longer dumps the EDID info like it did with 4.14). So I guess > the EDID I provided manually on the command line is not correctly put into > use with 4.19+ kernels. > > The 4.19 dmesg indicated that drm_kms_helper.edid_firmware is deprecated > so I also tested with drm.edid_firmware it suggested as the replacement > but with no luck (but I believe also the drm_kms_helper one should have > worked as it was only "deprecated"). > > I also tried 5.1.2 but it did not work any better (and with it also tried > removing all the manual *.edid_firmware from the command line so I still > need to provide one manually to have it reliable working it seems). This should be fixed by commit commit 48eaeb7664c76139438724d520a1ea4a84a3ed92 Author: Jani Nikula Date: Mon Jun 10 12:30:54 2019 +0300 drm: add fallback override/firmware EDID modes workaround in drm-misc-fixes, cc: stable v4.15. Thanks for the report. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110904] [VKMS] vblank counter error
https://bugs.freedesktop.org/show_bug.cgi?id=110904 Bug ID: 110904 Summary: [VKMS] vblank counter error Product: DRI Version: DRI git Hardware: Other OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: DRM/other Assignee: dri-devel@lists.freedesktop.org Reporter: oleg.vasi...@intel.com VKMS uses vblank simulation through hrtimer. Updates to vblank counter in DRM are performed through drm_update_vblank_count, where the elapsed time in ns divided by vblank period. VKMS vblank timestamp computation approach results in negative elapsed time, which leads to trash u64 division result. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 3/9] mm: Add write-protect and clean utilities for address space ranges
On Wed, Jun 12, 2019 at 08:42:37AM +0200, Thomas Hellström (VMware) wrote: > From: Thomas Hellstrom > > Add two utilities to a) write-protect and b) clean all ptes pointing into > a range of an address space. > The utilities are intended to aid in tracking dirty pages (either > driver-allocated system memory or pci device memory). > The write-protect utility should be used in conjunction with > page_mkwrite() and pfn_mkwrite() to trigger write page-faults on page > accesses. Typically one would want to use this on sparse accesses into > large memory regions. The clean utility should be used to utilize > hardware dirtying functionality and avoid the overhead of page-faults, > typically on large accesses into small memory regions. Please use EXPORT_SYMBOL_GPL, just like for apply_to_page_range and friends. Also in general new core functionality like this should go along with the actual user, we don't need to repeat the hmm disaster.
Re: [PATCH v3 08/10] arm64: dts: renesas: r8a7799[05]: Point LVDS0 to its companion LVDS1
On Wed, Jun 12, 2019 at 01:21:24PM +0300, Laurent Pinchart wrote: > Hi Simon, > > On Thu, Jun 06, 2019 at 10:51:09AM +0200, Simon Horman wrote: > > On Thu, Jun 06, 2019 at 10:59:57AM +0300, Laurent Pinchart wrote: > > > On Mon, Jun 03, 2019 at 01:40:45PM +0200, Simon Horman wrote: > > >> On Tue, May 28, 2019 at 05:12:32PM +0300, Laurent Pinchart wrote: > > >>> Add the new renesas,companion property to the LVDS0 node to point to the > > >>> companion LVDS encoder LVDS1. > > >>> > > >>> Signed-off-by: Laurent Pinchart > > >>> > > >>> Reviewed-by: Jacopo Mondi > > >>> Tested-by: Jacopo Mondi > > >> > > >> Hi Laurent, > > >> > > >> is this patch ready to be merged? > > > > > > I was hoping for an ack from the DT bindings maintainers for the > > > corresponding bindings changes, but I want to get this merged for the > > > next kernel release, so I may not get it. I'll ping you when I send the > > > pull request for the DT bindings and drivers changes, at that point this > > > patch should be picked up. > > > > Thanks Laurent, got it. > > The DT bindings and driver changes have been merged, so please go ahead > and pick this patch for v5.3. Thanks Laurent, done.
Re: [PATCH v5 3/9] mm: Add write-protect and clean utilities for address space ranges
On Wed, Jun 12, 2019 at 04:23:50AM -0700, Christoph Hellwig wrote: > friends. Also in general new core functionality like this should go > along with the actual user, we don't need to repeat the hmm disaster. Ok, I see you actually did that, it just got hidden by the awful selective cc stuff a lot of people do at the moment. Sorry for the noise.
Re: [PATCH v5 2/9] mm: Add an apply_to_pfn_range interface
On Wed, Jun 12, 2019 at 08:42:36AM +0200, Thomas Hellström (VMware) wrote:
> From: Thomas Hellstrom
>
> This is basically apply_to_page_range with added functionality:
> Allocating missing parts of the page table becomes optional, which
> means that the function can be guaranteed not to error if allocation
> is disabled. Also passing of the closure struct and callback function
> becomes different and more in line with how things are done elsewhere.
>
> Finally we keep apply_to_page_range as a wrapper around apply_to_pfn_range
>
> The reason for not using the page-walk code is that we want to perform
> the page-walk on vmas pointing to an address space without requiring the
> mmap_sem to be held rather than on vmas belonging to a process with the
> mmap_sem held.
>
> Notable changes since RFC:
> Don't export apply_to_pfn range.
>
> Cc: Andrew Morton
> Cc: Matthew Wilcox
> Cc: Will Deacon
> Cc: Peter Zijlstra
> Cc: Rik van Riel
> Cc: Minchan Kim
> Cc: Michal Hocko
> Cc: Huang Ying
> Cc: Souptick Joarder
> Cc: "Jérôme Glisse"
> Cc: linux...@kvack.org
> Cc: linux-ker...@vger.kernel.org
>
> Signed-off-by: Thomas Hellstrom
> Reviewed-by: Ralph Campbell #v1
> ---
> include/linux/mm.h | 10
> mm/memory.c| 135 ++---
> 2 files changed, 113 insertions(+), 32 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 0e8834ac32b7..3d06ce2a64af 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2675,6 +2675,16 @@ typedef int (*pte_fn_t)(pte_t *pte, pgtable_t token,
> unsigned long addr,
> extern int apply_to_page_range(struct mm_struct *mm, unsigned long address,
> unsigned long size, pte_fn_t fn, void *data);
>
> +struct pfn_range_apply;
> +typedef int (*pter_fn_t)(pte_t *pte, pgtable_t token, unsigned long addr,
> + struct pfn_range_apply *closure);
> +struct pfn_range_apply {
> + struct mm_struct *mm;
> + pter_fn_t ptefn;
> + unsigned int alloc;
> +};
> +extern int apply_to_pfn_range(struct pfn_range_apply *closure,
> + unsigned long address, unsigned long size);
>
> #ifdef CONFIG_PAGE_POISONING
> extern bool page_poisoning_enabled(void);
> diff --git a/mm/memory.c b/mm/memory.c
> index 168f546af1ad..462aa47f8878 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2032,18 +2032,17 @@ int vm_iomap_memory(struct vm_area_struct *vma,
> phys_addr_t start, unsigned long
> }
> EXPORT_SYMBOL(vm_iomap_memory);
>
> -static int apply_to_pte_range(struct mm_struct *mm, pmd_t *pmd,
> - unsigned long addr, unsigned long end,
> - pte_fn_t fn, void *data)
> +static int apply_to_pte_range(struct pfn_range_apply *closure, pmd_t *pmd,
> + unsigned long addr, unsigned long end)
> {
> pte_t *pte;
> int err;
> pgtable_t token;
> spinlock_t *uninitialized_var(ptl);
>
> - pte = (mm == &init_mm) ?
> + pte = (closure->mm == &init_mm) ?
> pte_alloc_kernel(pmd, addr) :
> - pte_alloc_map_lock(mm, pmd, addr, &ptl);
> + pte_alloc_map_lock(closure->mm, pmd, addr, &ptl);
> if (!pte)
> return -ENOMEM;
>
> @@ -2054,86 +2053,109 @@ static int apply_to_pte_range(struct mm_struct *mm,
> pmd_t *pmd,
> token = pmd_pgtable(*pmd);
>
> do {
> - err = fn(pte++, token, addr, data);
> + err = closure->ptefn(pte++, token, addr, closure);
> if (err)
> break;
> } while (addr += PAGE_SIZE, addr != end);
>
> arch_leave_lazy_mmu_mode();
>
> - if (mm != &init_mm)
> + if (closure->mm != &init_mm)
> pte_unmap_unlock(pte-1, ptl);
> return err;
> }
>
> -static int apply_to_pmd_range(struct mm_struct *mm, pud_t *pud,
> - unsigned long addr, unsigned long end,
> - pte_fn_t fn, void *data)
> +static int apply_to_pmd_range(struct pfn_range_apply *closure, pud_t *pud,
> + unsigned long addr, unsigned long end)
> {
> pmd_t *pmd;
> unsigned long next;
> - int err;
> + int err = 0;
>
> BUG_ON(pud_huge(*pud));
>
> - pmd = pmd_alloc(mm, pud, addr);
> + pmd = pmd_alloc(closure->mm, pud, addr);
> if (!pmd)
> return -ENOMEM;
> +
> do {
> next = pmd_addr_end(addr, end);
> - err = apply_to_pte_range(mm, pmd, addr, next, fn, data);
> + if (!closure->alloc && pmd_none_or_clear_bad(pmd))
> + continue;
> + err = apply_to_pte_range(closure, pmd, addr, next);
> if (err)
> break;
> } while (pmd++, addr = next, addr != end);
> return err;
> }
>
> -static int apply_to_pud_range(struct mm_struct *mm, p4d_t *p4d
Re: [PATCH v5 3/9] mm: Add write-protect and clean utilities for address space ranges
On 6/12/19 1:23 PM, Christoph Hellwig wrote: On Wed, Jun 12, 2019 at 08:42:37AM +0200, Thomas Hellström (VMware) wrote: From: Thomas Hellstrom Add two utilities to a) write-protect and b) clean all ptes pointing into a range of an address space. The utilities are intended to aid in tracking dirty pages (either driver-allocated system memory or pci device memory). The write-protect utility should be used in conjunction with page_mkwrite() and pfn_mkwrite() to trigger write page-faults on page accesses. Typically one would want to use this on sparse accesses into large memory regions. The clean utility should be used to utilize hardware dirtying functionality and avoid the overhead of page-faults, typically on large accesses into small memory regions. Please use EXPORT_SYMBOL_GPL, just like for apply_to_page_range and friends. Sounds reasonable if this uses already EXPORT_SYMBOL_GPL'd functionality. I'll respin. Also in general new core functionality like this should go along with the actual user, we don't need to repeat the hmm disaster. I see in your later message that you noticed the other patches. There's also user-space functionality in mesa that excercises this. /Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 2/9] mm: Add an apply_to_pfn_range interface
On 6/12/19 2:16 PM, Christoph Hellwig wrote:
On Wed, Jun 12, 2019 at 08:42:36AM +0200, Thomas Hellström (VMware) wrote:
From: Thomas Hellstrom
This is basically apply_to_page_range with added functionality:
Allocating missing parts of the page table becomes optional, which
means that the function can be guaranteed not to error if allocation
is disabled. Also passing of the closure struct and callback function
becomes different and more in line with how things are done elsewhere.
Finally we keep apply_to_page_range as a wrapper around apply_to_pfn_range
The reason for not using the page-walk code is that we want to perform
the page-walk on vmas pointing to an address space without requiring the
mmap_sem to be held rather than on vmas belonging to a process with the
mmap_sem held.
Notable changes since RFC:
Don't export apply_to_pfn range.
Cc: Andrew Morton
Cc: Matthew Wilcox
Cc: Will Deacon
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Minchan Kim
Cc: Michal Hocko
Cc: Huang Ying
Cc: Souptick Joarder
Cc: "Jérôme Glisse"
Cc: linux...@kvack.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Thomas Hellstrom
Reviewed-by: Ralph Campbell #v1
---
include/linux/mm.h | 10
mm/memory.c| 135 ++---
2 files changed, 113 insertions(+), 32 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0e8834ac32b7..3d06ce2a64af 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2675,6 +2675,16 @@ typedef int (*pte_fn_t)(pte_t *pte, pgtable_t token,
unsigned long addr,
extern int apply_to_page_range(struct mm_struct *mm, unsigned long address,
unsigned long size, pte_fn_t fn, void *data);
+struct pfn_range_apply;
+typedef int (*pter_fn_t)(pte_t *pte, pgtable_t token, unsigned long addr,
+struct pfn_range_apply *closure);
+struct pfn_range_apply {
+ struct mm_struct *mm;
+ pter_fn_t ptefn;
+ unsigned int alloc;
+};
+extern int apply_to_pfn_range(struct pfn_range_apply *closure,
+ unsigned long address, unsigned long size);
#ifdef CONFIG_PAGE_POISONING
extern bool page_poisoning_enabled(void);
diff --git a/mm/memory.c b/mm/memory.c
index 168f546af1ad..462aa47f8878 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2032,18 +2032,17 @@ int vm_iomap_memory(struct vm_area_struct *vma,
phys_addr_t start, unsigned long
}
EXPORT_SYMBOL(vm_iomap_memory);
-static int apply_to_pte_range(struct mm_struct *mm, pmd_t *pmd,
-unsigned long addr, unsigned long end,
-pte_fn_t fn, void *data)
+static int apply_to_pte_range(struct pfn_range_apply *closure, pmd_t *pmd,
+ unsigned long addr, unsigned long end)
{
pte_t *pte;
int err;
pgtable_t token;
spinlock_t *uninitialized_var(ptl);
- pte = (mm == &init_mm) ?
+ pte = (closure->mm == &init_mm) ?
pte_alloc_kernel(pmd, addr) :
- pte_alloc_map_lock(mm, pmd, addr, &ptl);
+ pte_alloc_map_lock(closure->mm, pmd, addr, &ptl);
if (!pte)
return -ENOMEM;
@@ -2054,86 +2053,109 @@ static int apply_to_pte_range(struct mm_struct *mm, pmd_t *pmd,
token = pmd_pgtable(*pmd);
do {
- err = fn(pte++, token, addr, data);
+ err = closure->ptefn(pte++, token, addr, closure);
if (err)
break;
} while (addr += PAGE_SIZE, addr != end);
arch_leave_lazy_mmu_mode();
- if (mm != &init_mm)
+ if (closure->mm != &init_mm)
pte_unmap_unlock(pte-1, ptl);
return err;
}
-static int apply_to_pmd_range(struct mm_struct *mm, pud_t *pud,
-unsigned long addr, unsigned long end,
-pte_fn_t fn, void *data)
+static int apply_to_pmd_range(struct pfn_range_apply *closure, pud_t *pud,
+ unsigned long addr, unsigned long end)
{
pmd_t *pmd;
unsigned long next;
- int err;
+ int err = 0;
BUG_ON(pud_huge(*pud));
- pmd = pmd_alloc(mm, pud, addr);
+ pmd = pmd_alloc(closure->mm, pud, addr);
if (!pmd)
return -ENOMEM;
+
do {
next = pmd_addr_end(addr, end);
- err = apply_to_pte_range(mm, pmd, addr, next, fn, data);
+ if (!closure->alloc && pmd_none_or_clear_bad(pmd))
+ continue;
+ err = apply_to_pte_range(closure, pmd, addr, next);
if (err)
break;
} while (pmd++, addr = next, addr != end);
return err;
}
-static int apply_to_pud_range(struct mm_struct *mm, p4d_t *p4d,
-unsigned long addr, unsigned long end,
-pte_f
Re: [PATCH v2 1/9] drm/gem-vram: Support pinning buffers to current location
On Wed, Jun 12, 2019 at 10:29:29AM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 12.06.19 um 10:13 schrieb Gerd Hoffmann:
> > On Tue, Jun 11, 2019 at 03:03:36PM +0200, Thomas Zimmermann wrote:
> >> Pinning a buffer prevents it from being moved to a different memory
> >> location. For some operations, such as buffer updates, it is not
> >> important where the buffer is located. Setting the pin function's
> >> pl_flag argument to 0 will pin the buffer to whereever it is stored.
> >>
> >> Signed-off-by: Thomas Zimmermann
> >> ---
> >> drivers/gpu/drm/drm_gem_vram_helper.c | 12
> >> 1 file changed, 8 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
> >> b/drivers/gpu/drm/drm_gem_vram_helper.c
> >> index 42ad80888df7..214f54b8920b 100644
> >> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> >> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> >> @@ -224,7 +224,9 @@ EXPORT_SYMBOL(drm_gem_vram_offset);
> >> *
> >> * Pinning a buffer object ensures that it is not evicted from
> >> * a memory region. A pinned buffer object has to be unpinned before
> >> - * it can be pinned to another region.
> >> + * it can be pinned to another region. If the pl_flag argument is 0,
> >> + * the buffer is pinned at its current location (video RAM or system
> >> + * memory).
> >> *
> >> * Returns:
> >> * 0 on success, or
> >> @@ -242,7 +244,9 @@ int drm_gem_vram_pin(struct drm_gem_vram_object *gbo,
> >> unsigned long pl_flag)
> >>if (gbo->pin_count)
> >>goto out;
> >>
> >> - drm_gem_vram_placement(gbo, pl_flag);
> >> + if (pl_flag)
> >> + drm_gem_vram_placement(gbo, pl_flag);
> >> +
> >>for (i = 0; i < gbo->placement.num_placement; ++i)
> >>gbo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT;
> >>
> >> @@ -691,7 +695,7 @@ int drm_gem_vram_driver_gem_prime_pin(struct
> >> drm_gem_object *gem)
> >> {
> >>struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem);
> >>
> >> - return drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
> >> + return drm_gem_vram_pin(gbo, 0);
>
> The only use case for these Prime helpers is fbdev console emulation. I
> have another patch set that replaces the ast and mgag200 consoles with
> generic code. During the console updates it temporarily pins the BO via
> this Prime funcation, which might move the BO into scarce VRAM
> unnecessarily.
Ok, if the pin is temporary only for the update this should be fine.
> Can we leave it like this and add a comment explaining
> the decision?
Yes, a comment is a good idea.
cheers,
Gerd
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/omapdrm: drop fb_debug_enter/leave
On Wed, Jun 12, 2019 at 11:19 AM Tomi Valkeinen wrote: > > On 12/06/2019 12:12, Daniel Vetter wrote: > > This is a no-op on atomic drivers because with atomic it's simply too > > complicated to get all the locking and workers and nonblocking > > synchronization correct, from essentially an NMI context. Well, too > > complicated = impossible. Also, omapdrm never implemented the > > mode_set_base_atomic hook, so I kinda wonder why this was ever added. > > > > Drop the hooks. > > f9b34a0fa4e25d9c0b72f124680c37c0c38f9934 > > It was just open coding DRM_FB_HELPER_DEFAULT_OPS, to get rid of > "Initializer entry defined twice" warning. > > Acked-by: Tomi Valkeinen > > Or I can pick it up to my branch if this is not part of a bigger series. Not part of a bigger series, there's still a bunch of real users of this in-tree. Although given how absolutely no one seems to care to make kgdb work on top of atomic drivers I'm hoping I can sunset all this code soon. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] drm/connector: Split out orientation quirk detection
Hi, On 12-06-19 02:16, dbasehore . wrote: On Tue, Jun 11, 2019 at 1:54 AM Hans de Goede wrote: Hi, On 11-06-19 10:08, Jani Nikula wrote: On Mon, 10 Jun 2019, Derek Basehore wrote: This removes the orientation quirk detection from the code to add an orientation property to a panel. This is used only for legacy x86 systems, yet we'd like to start using this on devicetree systems where quirk detection like this is not needed. Not needed, but no harm done either, right? I guess I'll defer judgement on this to Hans and Ville (Cc'd). Hmm, I'm not big fan of this change. It adds code duplication and as other models with the same issue using a different driver or panel-type show up we will get more code duplication. Also I'm not convinced that devicetree based platforms will not need this. The whole devicetree as an ABI thing, which means that all devicetree bindings need to be set in stone before things are merged into the mainline, is done solely so that we can get vendors to ship hardware with the dtb files included in the firmware. We've posted fixes to the devicetree well after the initial merge into mainline before, so I don't see what you mean about the bindings being set in stone. That was just me repeating the official party line about devicetree. I also don't really see the point. The devicetree is in the kernel. If there's some setting in the devicetree that we want to change, it's effectively the same to make the change in the devicetree versus some quirk setting. The only difference seems to be that making the change in the devicetree is cleaner. I agree with you that devicetree in practice is easy to update after shipping. But at least whenever I tried to get new bindings reviewed I was always told that I was not allowed to count on that. I'm 100% sure that there is e.g. ARM hardware out there which uses non upright mounted LCD panels (I used to have a few Allwinner tablets which did this). And given my experience with the quality of firmware bundled tables like ACPI tables I'm quite sure that if we ever move to firmware included dtb files that we will need quirks for those too. Is there a timeline to start using firmware bundled tables? Nope, as I said "if we ever move to ...". Since the quirk code only uses DMI, it will need to be changed anyways for firmware bundled devicetree files anyways. We could consolidate the duplicated code into another function that calls drm_get_panel_orientation_quirks too. The only reason it's like it is is because I initially only had the call to drm_get_panel_orientation_quirk once in the code. Yes if you can add a new helper for the current callers, then I'm fine with dropping the quirk handling from drm_connector_init_panel_orientation_property() Regards, Hans ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 00/15] tc358767 driver improvements
Hi, On 12/06/2019 11:32, Andrey Smirnov wrote: Everyone: This series contains various improvements (at least in my mind) and fixes that I made to tc358767 while working with the code of the driver. Hopefuly each patch is self explanatory. I haven't had time to debug, but I did a quick test with the series, and EDID read no longer works on my setup. drm_get_edid() returns 0. I need to dig in further. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 3/6] drm/modes: Allow to specify rotation and reflection on the commandline
Den 11.06.2019 14.49, skrev Maxime Ripard:
> Hi Noralf,
>
> On Thu, Apr 18, 2019 at 06:40:42PM +0200, Noralf Trønnes wrote:
>> Den 18.04.2019 14.41, skrev Maxime Ripard:
>>> + /**
>>> +* We want the rotation on the command line to overwrite
>>> +* whatever comes from the panel.
>>> +*/
>>> + cmdline = &connector->cmdline_mode;
>>> + if (cmdline->specified &&
>>> + cmdline->rotation != DRM_MODE_ROTATE_0)
>>
>> I believe you need to drop that second check, otherwise rotate=0 will
>> not overwrite panel rotation.
>
> Good catch :)
>
>>> + } else if (!strncmp(option, "reflect_x", delim - option)) {
>>> + rotation |= DRM_MODE_REFLECT_X;
>>> + sep = delim;
>>> + } else if (!strncmp(option, "reflect_y", delim - option)) {
>>
>> I think you should drop reflect_x and _y for now since they're not
>> supported. People like me that only reads code and not documentation
>> (ahem..) will get the impression that this should work.
>
> I'm not sure what you mean here, this is definitely supposed to
> work. Is there a limitation you're thinking of?
>
It's this one in drm_client_panel_rotation() which limits rotation to
DRM_MODE_ROTATE_180:
/*
* TODO: support 90 / 270 degree hardware rotation,
* depending on the hardware this may require the framebuffer
* to be in a specific tiling format.
*/
if (*rotation != DRM_MODE_ROTATE_180 || !plane->rotation_property)
return false;
Noralf.
>> Documentation/fb/modedb.txt needs to be updated with this new video= option.
>
> Will do, thanks!
>
> maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 10/15] drm/bridge: tc358767: Add support for address-only I2C transfers
Hi, On 12/06/2019 11:32, Andrey Smirnov wrote: Transfer size of zero means a request to do an address-only transfer. Since the HW support this, we probably shouldn't be just ignoring such requests. While at it allow DP_AUX_I2C_MOT flag to pass through, since it is supported by the HW as well. I bisected the EDID read issue to this patch... Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: [RFC PATCH] drm/panfrost: Add support for mapping BOs on GPU page faults
On Mon, 10 Jun 2019 at 19:06, Rob Herring wrote:
>
> The midgard/bifrost GPUs need to allocate GPU memory which is allocated
> on GPU page faults and not pinned in memory. The vendor driver calls
> this functionality GROW_ON_GPF.
>
> This implementation assumes that BOs allocated with the
> PANFROST_BO_NOMAP flag are never mmapped or exported. Both of those may
> actually work, but I'm unsure if there's some interaction there. It
> would cause the whole object to be pinned in memory which would defeat
> the point of this.
>
> Issues/questions/thoughts:
>
> What's the difference between i_mapping and f_mapping?
>
> What kind of clean-up on close is needed? Based on vgem faults, there
> doesn't seem to be any refcounting. Assume userspace is responsible for
> not freeing the BO while a page fault can occur?
Aren't we taking a reference on all BOs that a job relates to and
unreferencing them once the job is done? I would think that that's
enough, or am I missing something?
> What about evictions? Need to call mapping_set_unevictable()? Maybe we
> want these pages to be swappable, but then we need some notification to
> unmap them.
I'm not sure there's much point in swapping out pages with lifetimes
of a few milliseconds.
> No need for runtime PM, because faults should only occur during job
> submit?
Makes sense to me.
> Need to fault in 2MB sections when possible. It seems this has to be
> done by getting an array of struct pages and then converting to a
> scatterlist. Not sure if there is a better way to do that. There's also
> some errata on T604 needing to fault in at least 8 pages at a time which
> isn't handled.
The old GPUs will be the most interesting to support...
Will give this patch some testing tomorrow.
Thanks,
Tomeu
> Cc: Robin Murphy
> Cc: Steven Price
> Cc: Alyssa Rosenzweig
> Cc: Tomeu Vizoso
> Signed-off-by: Rob Herring
> ---
>
> drivers/gpu/drm/panfrost/TODO | 2 -
> drivers/gpu/drm/panfrost/panfrost_drv.c | 22 +--
> drivers/gpu/drm/panfrost/panfrost_gem.c | 3 +-
> drivers/gpu/drm/panfrost/panfrost_gem.h | 8 +++
> drivers/gpu/drm/panfrost/panfrost_mmu.c | 76 +++--
> include/uapi/drm/panfrost_drm.h | 2 +
> 6 files changed, 100 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/panfrost/TODO b/drivers/gpu/drm/panfrost/TODO
> index c2e44add37d8..64129bf73933 100644
> --- a/drivers/gpu/drm/panfrost/TODO
> +++ b/drivers/gpu/drm/panfrost/TODO
> @@ -14,8 +14,6 @@
>The hard part is handling when more address spaces are needed than what
>the h/w provides.
>
> -- Support pinning pages on demand (GPU page faults).
> -
> - Support userspace controlled GPU virtual addresses. Needed for Vulkan.
> (Tomeu)
>
> - Support for madvise and a shrinker.
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
> b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index d11e2281dde6..f08d41d5186a 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -44,9 +44,11 @@ static int panfrost_ioctl_create_bo(struct drm_device
> *dev, void *data,
> {
> int ret;
> struct drm_gem_shmem_object *shmem;
> + struct panfrost_gem_object *bo;
> struct drm_panfrost_create_bo *args = data;
>
> - if (!args->size || args->flags || args->pad)
> + if (!args->size || args->pad ||
> + (args->flags & ~PANFROST_BO_NOMAP))
> return -EINVAL;
>
> shmem = drm_gem_shmem_create_with_handle(file, dev, args->size,
> @@ -54,11 +56,16 @@ static int panfrost_ioctl_create_bo(struct drm_device
> *dev, void *data,
> if (IS_ERR(shmem))
> return PTR_ERR(shmem);
>
> - ret = panfrost_mmu_map(to_panfrost_bo(&shmem->base));
> - if (ret)
> - goto err_free;
> + bo = to_panfrost_bo(&shmem->base);
>
> - args->offset = to_panfrost_bo(&shmem->base)->node.start << PAGE_SHIFT;
> + if (!(args->flags & PANFROST_BO_NOMAP)) {
> + ret = panfrost_mmu_map(bo);
> + if (ret)
> + goto err_free;
> + } else
> + bo->no_map = 1;
> +
> + args->offset = bo->node.start << PAGE_SHIFT;
>
> return 0;
>
> @@ -269,6 +276,11 @@ static int panfrost_ioctl_mmap_bo(struct drm_device
> *dev, void *data,
> return -ENOENT;
> }
>
> + if (to_panfrost_bo(gem_obj)->no_map) {
> + DRM_DEBUG("Can't mmap 'no_map' GEM BO %d\n", args->handle);
> + return -EINVAL;
> + }
> +
> ret = drm_gem_create_mmap_offset(gem_obj);
> if (ret == 0)
> args->offset = drm_vma_node_offset_addr(&gem_obj->vma_node);
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c
> b/drivers/gpu/drm/panfrost/panfrost_gem.c
> index 886875ae31d3..ed0b4f79610a 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gem.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
> @@ -19,7 +19,8 @@ stat
Re: [PATCH v3 3/6] drm/modes: Allow to specify rotation and reflection on the commandline
Den 11.06.2019 15.20, skrev Maxime Ripard:
> Hi Noralf,
>
> On Fri, Apr 19, 2019 at 10:53:28AM +0200, Noralf Trønnes wrote:
>> Den 18.04.2019 18.40, skrev Noralf Trønnes:
>>>
>>>
>>> Den 18.04.2019 14.41, skrev Maxime Ripard:
Rotations and reflections setup are needed in some scenarios to initialise
properly the initial framebuffer. Some drivers already had a bunch of
quirks to deal with this, such as either a private kernel command line
parameter (omapdss) or on the device tree (various panels).
In order to accomodate this, let's create a video mode parameter to deal
with the rotation and reflexion.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_client_modeset.c | 10 +++-
drivers/gpu/drm/drm_modes.c | 110 ++--
include/drm/drm_connector.h | 9 ++-
3 files changed, 109 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c
b/drivers/gpu/drm/drm_client_modeset.c
index f2869c82510c..15145d2c86d5 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -823,6 +823,7 @@ EXPORT_SYMBOL(drm_client_modeset_probe);
bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int
*rotation)
{
struct drm_connector *connector = modeset->connectors[0];
+ struct drm_cmdline_mode *cmdline;
struct drm_plane *plane = modeset->crtc->primary;
u64 valid_mask = 0;
unsigned int i;
@@ -844,6 +845,15 @@ bool drm_client_panel_rotation(struct drm_mode_set
*modeset, unsigned int *rotat
*rotation = DRM_MODE_ROTATE_0;
}
+ /**
+ * We want the rotation on the command line to overwrite
+ * whatever comes from the panel.
+ */
+ cmdline = &connector->cmdline_mode;
+ if (cmdline->specified &&
+ cmdline->rotation != DRM_MODE_ROTATE_0)
>>>
>>> I believe you need to drop that second check, otherwise rotate=0 will
>>> not overwrite panel rotation.
>>>
+ *rotation = cmdline->rotation;
>>
>> I remembered that you wanted this to propagate to DRM userspace. That's
>> not happening here.
>
> It's propated to the userspace through the plane's rotation property,
> I just checked.
>
Oh, so the rotation property stores its value in plane_state->rotation.
And this is set in: drm_client_modeset_commit_atomic() ->
drm_client_panel_rotation(): plane_state->rotation = rotation;
>> The only way I see for that to happen, is to set
>> ->panel_orientation. And to repeat myself, imo that makes
>> 'orientation' a better name for this video= option.
>
> orientation and rotation are two different things to me. The
> orientation of a panel for example is absolute, while the rotation is
> a transformation. In this particular case, I think that both the
> orientation and the rotation should be taken into account, with the
> orientation being the default state, and the hardware / panel will
> tell us that, while the rotation would be a transformation from that
> default to whatever the user wants.
>
> More importantly, the orientation is a property of the hardware (ie,
> how the display has been assembled), while the rotation is a software
> construct.
>
> And if the property being used to expose that is the rotation, I guess
> it would make sense to just use the same name and remain consistent.
>
Ok, I see. Based on this, I would assume that rotation would be relative
to the orientation, but I see that in this patch rotation doesn't take
orintation into account, it just overwrites it. I don't how userspace
deals with rotation on top of orientation.
Noralf.
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v1] backlight: Don't build support by default
b20c5249aa6a ("backlight: Fix compile error if CONFIG_FB is unset")
added 'default m' for BACKLIGHT_CLASS_DEVICE and LCD_CLASS_DEVICE.
Let's go back to not building support by default.
Signed-off-by: Marc Gonzalez
---
drivers/video/backlight/Kconfig | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 8b081d61773e..40676be2e46a 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -10,7 +10,6 @@ menu "Backlight & LCD device support"
#
config LCD_CLASS_DEVICE
tristate "Lowlevel LCD controls"
- default m
help
This framework adds support for low-level control of LCD.
Some framebuffer devices connect to platform-specific LCD modules
@@ -143,7 +142,6 @@ endif # LCD_CLASS_DEVICE
#
config BACKLIGHT_CLASS_DEVICE
tristate "Lowlevel Backlight controls"
- default m
help
This framework adds support for low-level control of the LCD
backlight. This includes support for brightness and power.
--
2.17.1
[PATCH] dma-fence/reservation: Markup rcu protected access for DEBUG_MUTEXES
Mark the access to reservation_object.fence as being protected to
silence sparse.
Signed-off-by: Chris Wilson
---
include/linux/reservation.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/reservation.h b/include/linux/reservation.h
index ee750765cc94..644a22dbe53b 100644
--- a/include/linux/reservation.h
+++ b/include/linux/reservation.h
@@ -216,8 +216,12 @@ reservation_object_unlock(struct reservation_object *obj)
{
#ifdef CONFIG_DEBUG_MUTEXES
/* Test shared fence slot reservation */
- if (obj->fence)
- obj->fence->shared_max = obj->fence->shared_count;
+ if (rcu_access_pointer(obj->fence)) {
+ struct reservation_object_list *fence =
+ reservation_object_get_list(obj);
+
+ fence->shared_max = fence->shared_count;
+ }
#endif
ww_mutex_unlock(&obj->lock);
}
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/10] drm/vkms: rework crc worker
Hi Daniel, First of all, thank you very much for your patchset. I tried to make a detailed review of your series, and you can see my comments in each patch. You’ll notice that I asked many things related to the DRM subsystem with the hope that I could learn a little bit more about DRM from your comments. Before you go through the review, I would like to start a discussion about the vkms race conditions. First, I have to admit that I did not understand the race conditions that you described before because I couldn’t reproduce them. Now, I'm suspecting that I could not experience the problem because I'm using QEMU with KVM; with this idea in mind, I suppose that we have two scenarios for using vkms in a virtual machine: * Scenario 1: The user has hardware virtualization support; in this case, it is a little bit harder to experience race conditions with vkms. * Scenario 2: Without hardware virtualization support, it is much easier to experience race conditions. With these two scenarios in mind, I conducted a bunch of experiments for trying to shed light on this issue. I did: 1. Enabled lockdep 2. Defined two different environments for testing both using QEMU with and without kvm. See below the QEMU hardware options: * env_kvm: -enable-kvm -daemonize -m 1G -smp cores=2,cpus=2 * env_no_kvm: -daemonize -m 2G -smp cores=4,cpus=4 3. My test protocol: I) turn on the vm, II) clean /proc/lock_stat, III) execute kms_cursor_crc, III) save the lock_stat file, and IV) turn off the vm. 4. From the lockdep_stat, I just highlighted the row related to vkms and the columns holdtime-total and holdtime-avg I would like to highlight that the following data does not have any statistical approach, and its intention is solely to assist our discussion. See below the summary of the collected data: Summary of the experiment results: ++++ || env_kvm| env_no_kvm | ++++ | Test | Before | After | Before | After | +++---++---+ | kms_cursor_crc | S1 | S2 | M1 | M2 | +++---++---+ * Before: before apply this patchset * After: after apply this patchset -+--+--- S1: Without this patchset and with kvm | holdtime-total | holdtime-avg -++- &(&vkms_out->lock)->rlock: | 21983.52 | 6.21 (work_completion)(&vkms_state->crc_wo: | 20.47 | 20.47 (wq_completion)vkms_crc_workq: | 3999507.87| 3352.48 &(&vkms_out->state_lock)->rlock: | 378.47| 0.30 (work_completion)(&vkms_state->crc_#2: | 3999066.30| 2848.34 -++- S2: With this patchset and with kvm || -++- &(&vkms_out->lock)->rlock: | 23262.83 | 6.34 (work_completion)(&vkms_state->crc_wo: | 8.98 | 8.98 &(&vkms_out->crc_lock)->rlock: | 307.28| 0.32 (wq_completion)vkms_crc_workq: | 6567727.05| 12345.35 (work_completion)(&vkms_state->crc_#2: | 6567135.15| 4488.81 -++- M1: Without this patchset and without kvm|| -++- &(&vkms_out->state_lock)->rlock: | 4994.72 | 1.61 &(&vkms_out->lock)->rlock: | 247190.04 | 39.39 (work_completion)(&vkms_state->crc_wo: | 31.32 | 31.32 (wq_completion)vkms_crc_workq: | 20991073.78 | 13525.18 (work_completion)(&vkms_state->crc_#2: | 20988347.18 | 11904.90 -++- M2: With this patchset and without kvm || -++- (wq_completion)vkms_crc_workq: | 42819161.68 | 36597.57 &(&vkms_out->lock)->rlock: | 251257.06 | 35.80 (work_completion)(&vkms_state->crc_wo: | 69.37 | 69.37 &(&vkms_out->crc_lock)->rlock: | 3620.92 | 1.54 (work_completion)(&vkms_state->crc_#2: | 42803419.59 | 24306.31 First, I analyzed the scenarios with KVM (S1 and S2); more specifically, I focused on these two classes: 1. (work_completion)(&vkms_state->crc_wo 2. (work_completion)(&vkms_state->crc_#2 After taking a look at the data, it looks like that this patchset greatly reduces the hold time average for crc_wo. On the other hand, it increases the hold time average for crc_#2. I didn’t understand well the reason for the difference. Could you help me here? When I looked for the second set of scenarios (M1 and M2, bo
Re: [PATCH] dma-fence/reservation: Markup rcu protected access for DEBUG_MUTEXES
Am 12.06.19 um 15:28 schrieb Chris Wilson:
Mark the access to reservation_object.fence as being protected to
silence sparse.
Signed-off-by: Chris Wilson
Reviewed-by: Christian König
---
include/linux/reservation.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/reservation.h b/include/linux/reservation.h
index ee750765cc94..644a22dbe53b 100644
--- a/include/linux/reservation.h
+++ b/include/linux/reservation.h
@@ -216,8 +216,12 @@ reservation_object_unlock(struct reservation_object *obj)
{
#ifdef CONFIG_DEBUG_MUTEXES
/* Test shared fence slot reservation */
- if (obj->fence)
- obj->fence->shared_max = obj->fence->shared_count;
+ if (rcu_access_pointer(obj->fence)) {
+ struct reservation_object_list *fence =
+ reservation_object_get_list(obj);
+
+ fence->shared_max = fence->shared_count;
+ }
#endif
ww_mutex_unlock(&obj->lock);
}
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/mcde: Fix uninitialized variable
We need to handle the case when of_drm_find_bridge() returns
NULL.
Reported-by: Dan Carpenter
Cc: Dan Carpenter
Signed-off-by: Linus Walleij
---
drivers/gpu/drm/mcde/mcde_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mcde/mcde_drv.c b/drivers/gpu/drm/mcde/mcde_drv.c
index baf63fb6850a..bc11c446e594 100644
--- a/drivers/gpu/drm/mcde/mcde_drv.c
+++ b/drivers/gpu/drm/mcde/mcde_drv.c
@@ -319,7 +319,7 @@ static int mcde_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct drm_device *drm;
struct mcde *mcde;
- struct component_match *match;
+ struct component_match *match = NULL;
struct resource *res;
u32 pid;
u32 val;
@@ -485,7 +485,7 @@ static int mcde_probe(struct platform_device *pdev)
}
put_device(p);
}
- if (IS_ERR(match)) {
+ if (IS_ERR_OR_NULL(match)) {
dev_err(dev, "could not create component match\n");
ret = PTR_ERR(match);
goto clk_disable;
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/10] drm/vkms: Fix crc worker races
On Thu, Jun 6, 2019 at 7:28 PM Daniel Vetter wrote:
>
> The issue we have is that the crc worker might fall behind. We've
> tried to handle this by tracking both the earliest frame for which it
> still needs to compute a crc, and the last one. Plus when the
> crtc_state changes, we have a new work item, which are all run in
> order due to the ordered workqueue we allocate for each vkms crtc.
>
> Trouble is there's been a few small issues in the current code:
> - we need to capture frame_end in the vblank hrtimer, not in the
> worker. The worker might run much later, and then we generate a lot
> of crc for which there's already a different worker queued up.
> - frame number might be 0, so create a new crc_pending boolean to
> track this without confusion.
> - we need to atomically grab frame_start/end and clear it, so do that
> all in one go. This is not going to create a new race, because if we
> race with the hrtimer then our work will be re-run.
> - only race that can happen is the following:
> 1. worker starts
> 2. hrtimer runs and updates frame_end
> 3. worker grabs frame_start/end, already reading the new frame_end,
> and clears crc_pending
> 4. hrtimer calls queue_work()
> 5. worker completes
> 6. worker gets re-run, crc_pending is false
> Explain this case a bit better by rewording the comment.
>
> v2: Demote warning level output to debug when we fail to requeue, this
> is expected under high load when the crc worker can't quite keep up.
>
> Cc: Shayenne Moura
> Cc: Rodrigo Siqueira
> Signed-off-by: Daniel Vetter
> Cc: Haneen Mohammed
> Cc: Daniel Vetter
> ---
> drivers/gpu/drm/vkms/vkms_crc.c | 27 +--
> drivers/gpu/drm/vkms/vkms_crtc.c | 9 +++--
> drivers/gpu/drm/vkms/vkms_drv.h | 2 ++
> 3 files changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
> index d7b409a3c0f8..66603da634fe 100644
> --- a/drivers/gpu/drm/vkms/vkms_crc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crc.c
> @@ -166,16 +166,24 @@ void vkms_crc_work_handle(struct work_struct *work)
> struct drm_plane *plane;
> u32 crc32 = 0;
> u64 frame_start, frame_end;
> + bool crc_pending;
> unsigned long flags;
>
> spin_lock_irqsave(&out->state_lock, flags);
> frame_start = crtc_state->frame_start;
> frame_end = crtc_state->frame_end;
> + crc_pending = crtc_state->crc_pending;
> + crtc_state->frame_start = 0;
> + crtc_state->frame_end = 0;
> + crtc_state->crc_pending = false;
> spin_unlock_irqrestore(&out->state_lock, flags);
>
> - /* _vblank_handle() hasn't updated frame_start yet */
> - if (!frame_start || frame_start == frame_end)
> - goto out;
> + /*
> +* We raced with the vblank hrtimer and previous work already computed
> +* the crc, nothing to do.
> +*/
> + if (!crc_pending)
> + return;
I think this condition is not reachable because crc_pending will be
filled with true in `vkms_vblank_simulate()` which in turn schedule
the function `vkms_crc_work_handle()`. Just for checking, I tried to
reach this condition by running kms_flip, kms_pipe_crc_basic, and
kms_cursor_crc with two different VM setups[1], but I couldn't reach
it. What do you think?
[1] Qemu parameters
VM1: -m 1G -smp cores=2,cpus=2
VM2: -enable-kvm -m 2G -smp cores=4,cpus=4
> drm_for_each_plane(plane, &vdev->drm) {
> struct vkms_plane_state *vplane_state;
> @@ -196,20 +204,11 @@ void vkms_crc_work_handle(struct work_struct *work)
> if (primary_crc)
> crc32 = _vkms_get_crc(primary_crc, cursor_crc);
>
> - frame_end = drm_crtc_accurate_vblank_count(crtc);
> -
> - /* queue_work can fail to schedule crc_work; add crc for
> -* missing frames
> + /*
> +* The worker can fall behind the vblank hrtimer, make sure we catch
> up.
> */
> while (frame_start <= frame_end)
> drm_crtc_add_crc_entry(crtc, true, frame_start++, &crc32);
I want to take this opportunity to ask about this while; It's not
really specific to this patch.
I have to admit that I never fully got the idea behind this 'while';
it looks like that we just fill out the missed frames with a repeated
value. FWIU, `drm_crtc_add_crc_entry()` will add an entry with the CRC
information for a frame, but in this case, we are adding the same CRC
for a different set of frames. I agree that near frame has a similar
CRC value, but could we rely on this all the time? What could happen
if we have a great difference from the frame_start and frame_end?
> -
> -out:
> - /* to avoid using the same value for frame number again */
> - spin_lock_irqsave(&out->state_lock, flags);
> - crtc_state->frame_end = frame_end;
> - crtc_state->frame_start = 0;
> - spin_unlock_irqrestore(&out->state_lock, flags)
Re: [PATCH 02/10] drm/vkms: Use spin_lock_irq in process context
On Thu, Jun 6, 2019 at 7:28 PM Daniel Vetter wrote:
>
> The worker is always in process context, no need for the _irqsafe
> version. Same for the set_source callback, that's only called from the
> debugfs handler in a syscall.
>
> Cc: Shayenne Moura
> Cc: Rodrigo Siqueira
> Signed-off-by: Daniel Vetter
> Cc: Haneen Mohammed
> Cc: Daniel Vetter
> ---
> drivers/gpu/drm/vkms/vkms_crc.c | 10 --
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
> index 66603da634fe..883e36fe7b6e 100644
> --- a/drivers/gpu/drm/vkms/vkms_crc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crc.c
> @@ -167,16 +167,15 @@ void vkms_crc_work_handle(struct work_struct *work)
> u32 crc32 = 0;
> u64 frame_start, frame_end;
> bool crc_pending;
> - unsigned long flags;
>
> - spin_lock_irqsave(&out->state_lock, flags);
> + spin_lock_irq(&out->state_lock);
> frame_start = crtc_state->frame_start;
> frame_end = crtc_state->frame_end;
> crc_pending = crtc_state->crc_pending;
> crtc_state->frame_start = 0;
> crtc_state->frame_end = 0;
> crtc_state->crc_pending = false;
> - spin_unlock_irqrestore(&out->state_lock, flags);
> + spin_unlock_irq(&out->state_lock);
>
> /*
> * We raced with the vblank hrtimer and previous work already computed
> @@ -246,7 +245,6 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char
> *src_name)
> {
> struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
> bool enabled = false;
> - unsigned long flags;
> int ret = 0;
>
> ret = vkms_crc_parse_source(src_name, &enabled);
> @@ -254,9 +252,9 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char
> *src_name)
> /* make sure nothing is scheduled on crtc workq */
> flush_workqueue(out->crc_workq);
>
> - spin_lock_irqsave(&out->lock, flags);
> + spin_lock_irq(&out->lock);
> out->crc_enabled = enabled;
> - spin_unlock_irqrestore(&out->lock, flags);
I was wondering if we could use atomic_t for crc_enabled and avoid
this sort of lock. I did not try to change the data type; this is just
an idea.
Reviewed-by: Rodrigo Siqueira
Tested-by: Rodrigo Siqueira
> + spin_unlock_irq(&out->lock);
>
> return ret;
> }
> --
> 2.20.1
>
--
Rodrigo Siqueira
https://siqueira.tech
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/10] drm/vkms: Rename vkms_output.state_lock to crc_lock
On Thu, Jun 6, 2019 at 7:28 PM Daniel Vetter wrote:
>
> Plus add a comment about what it actually protects. It's very little.
>
> Signed-off-by: Daniel Vetter
> Cc: Rodrigo Siqueira
> Cc: Haneen Mohammed
> Cc: Daniel Vetter
> ---
> drivers/gpu/drm/vkms/vkms_crc.c | 4 ++--
> drivers/gpu/drm/vkms/vkms_crtc.c | 6 +++---
> drivers/gpu/drm/vkms/vkms_drv.h | 5 +++--
> 3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
> index 883e36fe7b6e..96806cd35ad4 100644
> --- a/drivers/gpu/drm/vkms/vkms_crc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crc.c
> @@ -168,14 +168,14 @@ void vkms_crc_work_handle(struct work_struct *work)
> u64 frame_start, frame_end;
> bool crc_pending;
>
> - spin_lock_irq(&out->state_lock);
> + spin_lock_irq(&out->crc_lock);
> frame_start = crtc_state->frame_start;
> frame_end = crtc_state->frame_end;
> crc_pending = crtc_state->crc_pending;
> crtc_state->frame_start = 0;
> crtc_state->frame_end = 0;
> crtc_state->crc_pending = false;
> - spin_unlock_irq(&out->state_lock);
> + spin_unlock_irq(&out->crc_lock);
>
> /*
> * We raced with the vblank hrtimer and previous work already computed
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c
> b/drivers/gpu/drm/vkms/vkms_crtc.c
> index c727d8486e97..55b16d545fe7 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -29,7 +29,7 @@ static enum hrtimer_restart vkms_vblank_simulate(struct
> hrtimer *timer)
> /* update frame_start only if a queued vkms_crc_work_handle()
> * has read the data
> */
> - spin_lock(&output->state_lock);
> + spin_lock(&output->crc_lock);
> if (!state->crc_pending)
> state->frame_start = frame;
> else
> @@ -37,7 +37,7 @@ static enum hrtimer_restart vkms_vblank_simulate(struct
> hrtimer *timer)
> state->frame_start, frame);
> state->frame_end = frame;
> state->crc_pending = true;
> - spin_unlock(&output->state_lock);
> + spin_unlock(&output->crc_lock);
>
> ret = queue_work(output->crc_workq, &state->crc_work);
> if (!ret)
> @@ -224,7 +224,7 @@ int vkms_crtc_init(struct drm_device *dev, struct
> drm_crtc *crtc,
> drm_crtc_helper_add(crtc, &vkms_crtc_helper_funcs);
>
> spin_lock_init(&vkms_out->lock);
> - spin_lock_init(&vkms_out->state_lock);
> + spin_lock_init(&vkms_out->crc_lock);
>
> vkms_out->crc_workq = alloc_ordered_workqueue("vkms_crc_workq", 0);
> if (!vkms_out->crc_workq)
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 3c7e06b19efd..43d3f98289fe 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -57,6 +57,7 @@ struct vkms_crtc_state {
> struct drm_crtc_state base;
> struct work_struct crc_work;
>
> + /* below three are protected by vkms_output.crc_lock */
> bool crc_pending;
> u64 frame_start;
> u64 frame_end;
> @@ -74,8 +75,8 @@ struct vkms_output {
> struct workqueue_struct *crc_workq;
> /* protects concurrent access to crc_data */
> spinlock_t lock;
It's not really specific to this patch, but after reviewing it, I was
thinking about the use of the 'lock' field in the struct vkms_output.
Do we really need it? It looks like that crc_lock just replaced it.
Additionally, going a little bit deeper in the lock field at struct
vkms_output, I was also asking myself: what critical area do we want
to protect with this lock? After inspecting the use of this lock, I
noticed two different places that use it:
1. In the function vkms_vblank_simulate()
Note that we cover a vast area with ‘output->lock’. IMHO we just need
to protect the state variable (line “state = output->crc_state;”) and
we can also use crc_lock for this case. Make sense?
2. In the function vkms_crtc_atomic_begin()
We also hold output->lock in the vkms_crtc_atomic_begin() and just
release it at vkms_crtc_atomic_flush(). Do we still need it?
> - /* protects concurrent access to crtc_state */
> - spinlock_t state_lock;
> +
> + spinlock_t crc_lock;
> };
Maybe add a kernel doc on top of crc_lock?
>
> struct vkms_device {
> --
> 2.20.1
>
--
Rodrigo Siqueira
https://siqueira.tech
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/10] drm/vkms: Move format arrays to vkms_plane.c
On Thu, Jun 6, 2019 at 7:28 PM Daniel Vetter wrote:
>
> No need to have them multiple times.
>
> Signed-off-by: Daniel Vetter
> Cc: Rodrigo Siqueira
> Cc: Haneen Mohammed
> Cc: Daniel Vetter
> ---
> drivers/gpu/drm/vkms/vkms_drv.h | 8
> drivers/gpu/drm/vkms/vkms_plane.c | 8
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 43d3f98289fe..2a35299bfb89 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -20,14 +20,6 @@
>
> extern bool enable_cursor;
>
> -static const u32 vkms_formats[] = {
> - DRM_FORMAT_XRGB,
> -};
> -
> -static const u32 vkms_cursor_formats[] = {
> - DRM_FORMAT_ARGB,
> -};
> -
> struct vkms_crc_data {
> struct drm_framebuffer fb;
> struct drm_rect src, dst;
> diff --git a/drivers/gpu/drm/vkms/vkms_plane.c
> b/drivers/gpu/drm/vkms/vkms_plane.c
> index 0e67d2d42f0c..0fceb6258422 100644
> --- a/drivers/gpu/drm/vkms/vkms_plane.c
> +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> @@ -6,6 +6,14 @@
> #include
> #include
>
> +static const u32 vkms_formats[] = {
> + DRM_FORMAT_XRGB,
> +};
> +
> +static const u32 vkms_cursor_formats[] = {
> + DRM_FORMAT_ARGB,
> +};
> +
> static struct drm_plane_state *
> vkms_plane_duplicate_state(struct drm_plane *plane)
> {
> --
> 2.20.1
>
Reviewed-by: Rodrigo Siqueira
Tested-by: Rodrigo Siqueira
--
Rodrigo Siqueira
https://siqueira.tech
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 06/10] drm/vkms: flush crc workers earlier in commit flow
On Thu, Jun 6, 2019 at 7:28 PM Daniel Vetter wrote:
>
> Currently we flush pending crc workers very late in the commit flow,
> when we destry all the old crtc states. Unfortunately at that point
destry -> destroy
> the framebuffers are already unpinned (and our vaddr possible gone),
> so this isn't good. Also, the plane_states we need might also already
> be cleaned up, since cleanup order of state structures isn't well
> defined.
>
> Fix this by waiting for all crc workers of the old state to complete
> before we start any of the cleanup work.
>
> Note that this is not yet race-free, because the hrtimer and crc
> worker look at the wrong state pointers, but that will be fixed in
> subsequent patches.
>
> Signed-off-by: Daniel Vetter
> Cc: Rodrigo Siqueira
> Cc: Haneen Mohammed
> Cc: Daniel Vetter
> ---
> drivers/gpu/drm/vkms/vkms_crtc.c | 2 +-
> drivers/gpu/drm/vkms/vkms_drv.c | 10 ++
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c
> b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 55b16d545fe7..b6987d90805f 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -125,7 +125,7 @@ static void vkms_atomic_crtc_destroy_state(struct
> drm_crtc *crtc,
> __drm_atomic_helper_crtc_destroy_state(state);
>
> if (vkms_state) {
> - flush_work(&vkms_state->crc_work);
> + WARN_ON(work_pending(&vkms_state->crc_work));
> kfree(vkms_state);
> }
> }
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index f677ab1d0094..cc53ef88a331 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -62,6 +62,9 @@ static void vkms_release(struct drm_device *dev)
> static void vkms_atomic_commit_tail(struct drm_atomic_state *old_state)
> {
> struct drm_device *dev = old_state->dev;
> + struct drm_crtc *crtc;
> + struct drm_crtc_state *old_crtc_state;
> + int i;
>
> drm_atomic_helper_commit_modeset_disables(dev, old_state);
>
> @@ -75,6 +78,13 @@ static void vkms_atomic_commit_tail(struct
> drm_atomic_state *old_state)
>
> drm_atomic_helper_wait_for_vblanks(dev, old_state);
>
> + for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i) {
> + struct vkms_crtc_state *vkms_state =
> + to_vkms_crtc_state(old_crtc_state);
> +
> + flush_work(&vkms_state->crc_work);
> + }
> +
> drm_atomic_helper_cleanup_planes(dev, old_state);
> }
why not use drm_atomic_helper_commit_tail() here? I mean:
for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i) {
…
}
drm_atomic_helper_commit_tail(old_state);
After looking at drm_atomic_helper_cleanup_planes() it sounds safe for
me to use the above code; I just test it with two tests from
crc_cursor. Maybe I missed something, could you help me here?
Finally, IMHO, I think that Patch 05, 06 and 07 could be squashed in a
single patch to make it easier to understand the change.
> --
> 2.20.1
>
--
Rodrigo Siqueira
https://siqueira.tech
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/mcde: Fix uninitialized variable
On Wed, Jun 12, 2019 at 03:30:38PM +0200, Linus Walleij wrote:
> We need to handle the case when of_drm_find_bridge() returns
> NULL.
>
> Reported-by: Dan Carpenter
> Cc: Dan Carpenter
> Signed-off-by: Linus Walleij
> ---
> drivers/gpu/drm/mcde/mcde_drv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/mcde/mcde_drv.c b/drivers/gpu/drm/mcde/mcde_drv.c
> index baf63fb6850a..bc11c446e594 100644
> --- a/drivers/gpu/drm/mcde/mcde_drv.c
> +++ b/drivers/gpu/drm/mcde/mcde_drv.c
> @@ -319,7 +319,7 @@ static int mcde_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct drm_device *drm;
> struct mcde *mcde;
> - struct component_match *match;
> + struct component_match *match = NULL;
> struct resource *res;
> u32 pid;
> u32 val;
> @@ -485,7 +485,7 @@ static int mcde_probe(struct platform_device *pdev)
> }
> put_device(p);
> }
> - if (IS_ERR(match)) {
> + if (IS_ERR_OR_NULL(match)) {
> dev_err(dev, "could not create component match\n");
> ret = PTR_ERR(match);
This doesn't work. If "match" is NULL then "ret" is zero which is
success.
> goto clk_disable;
regards,
dan carpenter
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 08/10] drm/vkms: No _irqsave within spin_lock_irq needed
On Thu, Jun 6, 2019 at 7:28 PM Daniel Vetter wrote:
>
> irqs are already off.
>
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/vkms/vkms_crtc.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c
> b/drivers/gpu/drm/vkms/vkms_crtc.c
> index b6987d90805f..48a793ba4030 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -183,17 +183,16 @@ static void vkms_crtc_atomic_flush(struct drm_crtc
> *crtc,
>struct drm_crtc_state *old_crtc_state)
> {
> struct vkms_output *vkms_output = drm_crtc_to_vkms_output(crtc);
> - unsigned long flags;
>
> if (crtc->state->event) {
> - spin_lock_irqsave(&crtc->dev->event_lock, flags);
> + spin_lock(&crtc->dev->event_lock);
>
> if (drm_crtc_vblank_get(crtc) != 0)
> drm_crtc_send_vblank_event(crtc, crtc->state->event);
> else
> drm_crtc_arm_vblank_event(crtc, crtc->state->event);
>
> - spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
> + spin_unlock(&crtc->dev->event_lock);
>
> crtc->state->event = NULL;
> }
> --
> 2.20.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Reviewed-by: Rodrigo Siqueira
Tested-by: Rodrigo Siqueira
--
Rodrigo Siqueira
https://siqueira.tech
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
