[PATCH] drm/nouveau: Fix missing pci_disable_device() in nouveau_pmops_runtime_resume()
Add missing pci_disable_device() in fail path of nouveau_do_resume().
Fixes: 7bb6d4428d3d ("drm/nouveau: move the (far too many...) different s/r
paths to the same place")
Signed-off-by: Shang XiaoJing
---
drivers/gpu/drm/nouveau/nouveau_drm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index fd99ec0f4257..d75cd21c1457 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1068,6 +1068,7 @@ nouveau_pmops_runtime_resume(struct device *dev)
ret = nouveau_do_resume(drm_dev, true);
if (ret) {
NV_ERROR(drm, "resume failed with: %d\n", ret);
+ pci_disable_device(pdev);
return ret;
}
--
2.17.1
Re: [PATCH v6 5/7] drm/bridge: anx7625: Register Type C mode switches
Hi Andy,
Thanks for reviewing the patch.
On Thu, Nov 24, 2022 at 8:18 PM Andy Shevchenko
wrote:
>
> On Thu, Nov 24, 2022 at 06:20:54PM +0800, Pin-yen Lin wrote:
> > Register USB Type-C mode switches when the "mode-switch" property and
> > relevant port are available in Device Tree. Configure the crosspoint
> > switch based on the entered alternate mode for a specific Type-C
> > connector.
>
> ...
>
> > +static void anx7625_typec_two_ports_update(struct anx7625_data *ctx)
> > +{
> > + if (ctx->typec_ports[0].dp_connected &&
> > ctx->typec_ports[1].dp_connected)
> > + /* Both ports available, do nothing to retain the current
> > one. */
> > + return;
>
> > + else if (ctx->typec_ports[0].dp_connected)
>
> This 'else' is redundant. I would rewrite above as
>
> /* Check if both ports available and do nothing to retain the current
> one */
> if (ctx->typec_ports[0].dp_connected &&
> ctx->typec_ports[1].dp_connected)
> return;
>
> if (ctx->typec_ports[0].dp_connected)
>
> > + anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_NORMAL);
> > + else if (ctx->typec_ports[1].dp_connected)
> > + anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_REVERSE);
> > +}
Thanks for the detailed suggestion. I'll adapt this in v7.
>
> ...
>
> > + data->dp_connected = (state->alt && state->alt->svid ==
> > USB_TYPEC_DP_SID &&
> > + state->alt->mode == USB_TYPEC_DP_MODE);
>
> Parentheses are not needed.
Will fix this in v7.
>
> ...
>
> > + /*
> > + * <0 1> refers to SSRX1/SSTX1, and <2 3> refers to SSRX2/SSTX2.
> > + */
> > + for (i = 0; i < num_lanes; i++) {
>
> > + if (port_num != -1 && port_num != dp_lanes[i] / 2) {
> > + dev_err(dev, "Invalid data lane numbers\n");
> > + return -EINVAL;
> > + }
>
> According to Rob Linux must not validate device tree. If you need it, use
> proper YAML schema.
>
> > + port_num = dp_lanes[i] / 2;
> > + }
>
I'll remove this from the driver in v7.
> ...
>
> > + if (!ctx->num_typec_switches) {
> > + dev_warn(dev, "No Type-C switches node found\n");
>
> > + return ret;
>
> Why not to return 0 explicitly?
Will update to just "return 0" in v7.
>
> > + }
>
> ...
>
> > + ctx->typec_ports = devm_kcalloc(
>
> Broken indentation.
Will fix in v7
>
> > + dev, ctx->num_typec_switches, sizeof(struct
> > anx7625_port_data),
> > + GFP_KERNEL);
> > + if (!ctx->typec_ports)
> > + return -ENOMEM;
>
> ...
>
> > +struct anx7625_port_data {
>
> > + bool dp_connected;
>
> You can save some bytes on some architectures if move this to be last field.
Thanks for the suggestion. I'll do so in v7
>
> > + struct typec_mux_dev *typec_mux;
> > + struct anx7625_data *ctx;
> > +};
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
Re: [PATCH v6 7/7] drm/bridge: it6505: Register Type C mode switches
Hi Andy,
Thanks for reviewing the patch.
On Thu, Nov 24, 2022 at 8:23 PM Andy Shevchenko
wrote:
>
> On Thu, Nov 24, 2022 at 06:20:56PM +0800, Pin-yen Lin wrote:
> > Register USB Type-C mode switches when the "mode-switch" property and
> > relevant port are available in Device Tree. Configure the "lane_swap"
> > state based on the entered alternate mode for a specific Type-C
> > connector, which ends up updating the lane swap registers of the it6505
> > chip.
>
> ...
>
> > config DRM_ITE_IT6505
> > tristate "ITE IT6505 DisplayPort bridge"
> > depends on OF
> > + depends on TYPEC || TYPEC=n
> > select DRM_DISPLAY_DP_HELPER
> > select DRM_DISPLAY_HDCP_HELPER
> > select DRM_DISPLAY_HELPER
>
> Something went wrong with the indentation. Perhaps you need to fix it first.
>
> ...
>
> > #include
> > #include
> > #include
> > +#include
>
> Make it ordered?
Will fix it in v7.
>
> ...
>
> > +struct it6505_port_data {
>
> > + bool dp_connected;
>
> Perhaps make it last?
Will fix it in v7.
>
> > + struct typec_mux_dev *typec_mux;
> > + struct it6505 *it6505;
> > +};
>
> ...
>
> > +static void it6505_typec_ports_update(struct it6505 *it6505)
> > +{
> > + usleep_range(3000, 4000);
> > +
> > + if (it6505->typec_ports[0].dp_connected &&
> > it6505->typec_ports[1].dp_connected)
> > + /* Both ports available, do nothing to retain the current
> > one. */
> > + return;
> > + else if (it6505->typec_ports[0].dp_connected)
> > + it6505->lane_swap = false;
> > + else if (it6505->typec_ports[1].dp_connected)
> > + it6505->lane_swap = true;
> > +
> > + usleep_range(3000, 4000);
> > +}
>
> As per previous patch comments.
Will update it in v7.
>
> Also, comment out these long sleeps. Why they are needed.
Actually, it turns out that these sleeps are not needed. I'll remove it in v7.
>
> ...
>
> > + int ret = pm_runtime_get_sync(dev);
> > +
> > + /*
> > + * On system resume, mux_set can be triggered before
> > + * pm_runtime_force_resume re-enables runtime power
> > management.
>
> We refer to the functions as func().
Will fix this in v7.
>
> > + * Handling the error here to make sure the bridge is powered
> > on.
> > + */
> > + if (ret < 0)
> > + it6505_poweron(it6505);
>
> This seems needed a bit more of explanation, esp. why you leave PM runtime
> reference count bumped up.
pm_runtime_force_suspend() disables runtime PM when the device enters
suspend, and sometime it6505_typec_mux_set() is called before
pm_runtime_force_resume brings runtime PM back. We force power up the
bridge in this case and leave the ref count incremented to make the
future pm_runtime_(get|put)_sync() calls balanced.
I'll include more explanations around this in v7.
>
> ...
>
> > + num_lanes = drm_of_get_data_lanes_count(node, 0, 2);
> > + if (num_lanes <= 0) {
> > + dev_err(dev, "Error on getting data lanes count: %d\n",
> > + num_lanes);
> > + return num_lanes;
> > + }
> > +
> > + ret = of_property_read_u32_array(node, "data-lanes", dp_lanes,
> > num_lanes);
> > + if (ret) {
> > + dev_err(dev, "Failed to read the data-lanes variable: %d\n",
> > + ret);
> > + return ret;
> > + }
> > +
> > + for (i = 0; i < num_lanes; i++) {
> > + if (port_num != -1 && port_num != dp_lanes[i] / 2) {
> > + dev_err(dev, "Invalid data lane numbers\n");
> > + return -EINVAL;
> > + }
>
> As per previous patch comments.
I'll remove this part in v7 and try to figure out how to do similar
checking with schemas.
>
> > + port_num = dp_lanes[i] / 2;
> > + }
>
> The above seems like tons of duplicating code that drivers need to implement.
> Can we shrink that burden by adding some library functions?
Could you advise where this lib file should go, and what the namings
can be? The "port-switching" logic is specific to some of the DP
bridges, and I'm not sure what kinds of naming/structure fit into this
case.
Thanks and regards,
Pin-yen
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
Re: [PATCH] drm/msm/dpu: Print interrupt index in addition to the mask
On 11/21/2022 2:24 PM, Marijn Suijten wrote:
The mask only describes the `irq_idx % 32` part, making it generally
impossible to deduce what interrupt is being enabled/disabled. Since
`debug/core_irq` in debugfs (and other prints) also include the full
`DPU_IRQ_IDX()` value, print the same full value here for easier
correlation instead of only adding the `irq_idx / 32` part.
Furthermore, make the dbgstr messages more consistent.
Signed-off-by: Marijn Suijten
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
index cf1b6d84c18a..64589a9c2c51 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
@@ -252,9 +252,9 @@ static int dpu_hw_intr_enable_irq_locked(struct dpu_hw_intr
*intr, int irq_idx)
cache_irq_mask = intr->cache_irq_mask[reg_idx];
if (cache_irq_mask & DPU_IRQ_MASK(irq_idx)) {
- dbgstr = "DPU IRQ already set:";
+ dbgstr = "already ";
} else {
- dbgstr = "DPU IRQ enabled:";
+ dbgstr = "";
cache_irq_mask |= DPU_IRQ_MASK(irq_idx);
/* Cleaning any pending interrupt */
@@ -268,7 +268,7 @@ static int dpu_hw_intr_enable_irq_locked(struct dpu_hw_intr
*intr, int irq_idx)
intr->cache_irq_mask[reg_idx] = cache_irq_mask;
}
- pr_debug("%s MASK:0x%.8lx, CACHE-MASK:0x%.8x\n", dbgstr,
+ pr_debug("DPU IRQ %d %senabled: MASK:0x%.8lx, CACHE-MASK:0x%.8x\n",
irq_idx, dbgstr,
DPU_IRQ_MASK(irq_idx), cache_irq_mask);
return 0;
@@ -301,9 +301,9 @@ static int dpu_hw_intr_disable_irq_locked(struct
dpu_hw_intr *intr, int irq_idx)
cache_irq_mask = intr->cache_irq_mask[reg_idx];
if ((cache_irq_mask & DPU_IRQ_MASK(irq_idx)) == 0) {
- dbgstr = "DPU IRQ is already cleared:";
+ dbgstr = "already ";
} else {
- dbgstr = "DPU IRQ mask disable:";
+ dbgstr = "";
cache_irq_mask &= ~DPU_IRQ_MASK(irq_idx);
/* Disable interrupts based on the new mask */
@@ -317,7 +317,7 @@ static int dpu_hw_intr_disable_irq_locked(struct
dpu_hw_intr *intr, int irq_idx)
intr->cache_irq_mask[reg_idx] = cache_irq_mask;
}
- pr_debug("%s MASK:0x%.8lx, CACHE-MASK:0x%.8x\n", dbgstr,
+ pr_debug("DPU IRQ %d %sdisabled: MASK:0x%.8lx, CACHE-MASK:0x%.8x\n",
irq_idx, dbgstr,
DPU_IRQ_MASK(irq_idx), cache_irq_mask);
return 0;
Re: [PATCH] drm/amdgpu: add mb for si
On 11/25/2022 7:43 AM, Quan, Evan wrote: [AMD Official Use Only - General] -Original Message- From: Lazar, Lijo Sent: Thursday, November 24, 2022 6:49 PM To: Quan, Evan ; 李真能 ; Michel Dänzer ; Koenig, Christian ; Deucher, Alexander Cc: amd-...@lists.freedesktop.org; Pan, Xinhui ; linux-ker...@vger.kernel.org; dri-devel@lists.freedesktop.org Subject: Re: [PATCH] drm/amdgpu: add mb for si On 11/24/2022 4:11 PM, Lazar, Lijo wrote: On 11/24/2022 3:34 PM, Quan, Evan wrote: [AMD Official Use Only - General] Could the attached patch help? Evan -Original Message- From: amd-gfx On Behalf Of ??? Sent: Friday, November 18, 2022 5:25 PM To: Michel Dänzer ; Koenig, Christian ; Deucher, Alexander Cc: amd-...@lists.freedesktop.org; Pan, Xinhui ; linux-ker...@vger.kernel.org; dri-devel@lists.freedesktop.org Subject: Re: [PATCH] drm/amdgpu: add mb for si 在 2022/11/18 17:18, Michel Dänzer 写道: On 11/18/22 09:01, Christian König wrote: Am 18.11.22 um 08:48 schrieb Zhenneng Li: During reboot test on arm64 platform, it may failure on boot, so add this mb in smc. The error message are as follows: [ 6.996395][ 7] [ T295] [drm:amdgpu_device_ip_late_init [amdgpu]] *ERROR* late_init of IP block failed -22 [ 7.006919][ 7] [ T295] amdgpu :04:00.0: The issue is happening in late_init() which eventually does ret = si_thermal_enable_alert(adev, false); Just before this, si_thermal_start_thermal_controller is called in hw_init and that enables thermal alert. Maybe the issue is with enable/disable of thermal alerts in quick succession. Adding a delay inside si_thermal_start_thermal_controller might help. On a second look, temperature range is already set as part of si_thermal_start_thermal_controller in hw_init https://elixir.bootlin.com/linux/v6.1- rc6/source/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c#L6780 There is no need to set it again here - https://elixir.bootlin.com/linux/v6.1- rc6/source/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c#L7635 I think it is safe to remove the call from late_init altogether. Alex/Evan? [Quan, Evan] Yes, it makes sense to me. But I'm not sure whether that’s related with the issue here. Since per my understandings, if the issue is caused by double calling of thermal_alert enablement, it will fail every time. That cannot explain why adding some delays or a mb() calling can help. The side effect of the patch is just some random delay introduced for every SMC message The issue happens in late_init(). Between late_init() and dpm enablement, there are many smc messages sent which don't have this issue. So I think the issue is not with FW not running. Thus the only case I see is enable/disable of thermal alert in random succession. Thanks, Lijo BR Evan Thanks, Lijo Thanks, Lijo amdgpu_device_ip_late_init failed [ 7.014224][ 7] [ T295] amdgpu :04:00.0: Fatal error during GPU init Memory barries are not supposed to be sprinkled around like this, you need to give a detailed explanation why this is necessary. Regards, Christian. Signed-off-by: Zhenneng Li --- drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c b/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c index 8f994ffa9cd1..c7656f22278d 100644 --- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c @@ -155,6 +155,8 @@ bool amdgpu_si_is_smc_running(struct amdgpu_device *adev) u32 rst = RREG32_SMC(SMC_SYSCON_RESET_CNTL); u32 clk = RREG32_SMC(SMC_SYSCON_CLOCK_CNTL_0); + mb(); + if (!(rst & RST_REG) && !(clk & CK_DISABLE)) return true; In particular, it makes no sense in this specific place, since it cannot directly affect the values of rst & clk. I thinks so too. But when I do reboot test using nine desktop machines, there maybe report this error on one or two machines after Hundreds of times or Thousands of times reboot test, at the beginning, I use msleep() instead of mb(), these two methods are all works, but I don't know what is the root case. I use this method on other verdor's oland card, this error message are reported again. What could be the root reason? test environmen: graphics card: OLAND 0x1002:0x6611 0x1642:0x1869 0x87 driver: amdgpu os: ubuntu 2004 platform: arm64 kernel: 5.4.18
Re: [Freedreno] [PATCH v2 03/12] drm/msm/dpu: Refactor sc7280_pp location
On 11/15/2022 5:33 AM, Konrad Dybcio wrote:
On 15/11/2022 14:30, Robert Foss wrote:
The sc7280_pp declaration is not located by the other _pp
declarations, but rather hidden around the _merge_3d
declarations. Let's fix this to avoid confusion.
Signed-off-by: Robert Foss
Reviewed-by: Dmitry Baryshkov
---
This is already merged.
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=1a5b5372e3b0a4cc65a0cbb724b1b0859f4ac63c
Konrad
Its part of linux-next but a PR hasnt been sent with it.
That being said, since this particular change has been taken separately,
this series should now be rebased without this change and addressing
some of the other comments given by konrad.
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 4dac90ee5b8a..8f2d634f7b6b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -1294,6 +1294,13 @@ static const struct dpu_pingpong_cfg
sm8150_pp[] = {
-1),
};
+static const struct dpu_pingpong_cfg sc7280_pp[] = {
+ PP_BLK("pingpong_0", PINGPONG_0, 0x59000, 0, sc7280_pp_sblk, -1,
-1),
+ PP_BLK("pingpong_1", PINGPONG_1, 0x6a000, 0, sc7280_pp_sblk, -1,
-1),
+ PP_BLK("pingpong_2", PINGPONG_2, 0x6b000, 0, sc7280_pp_sblk, -1,
-1),
+ PP_BLK("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk, -1,
-1),
+};
+
static struct dpu_pingpong_cfg qcm2290_pp[] = {
PP_BLK("pingpong_0", PINGPONG_0, 0x7, 0, sdm845_pp_sblk,
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
@@ -1352,13 +1359,6 @@ static const struct dpu_merge_3d_cfg
sm8450_merge_3d[] = {
MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x65f00),
};
-static const struct dpu_pingpong_cfg sc7280_pp[] = {
- PP_BLK("pingpong_0", PINGPONG_0, 0x59000, 0, sc7280_pp_sblk, -1,
-1),
- PP_BLK("pingpong_1", PINGPONG_1, 0x6a000, 0, sc7280_pp_sblk, -1,
-1),
- PP_BLK("pingpong_2", PINGPONG_2, 0x6b000, 0, sc7280_pp_sblk, -1,
-1),
- PP_BLK("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk, -1,
-1),
-};
-
/*
* DSC sub blocks config
*/
Re: [PATCH v5 08/10] drm/msm/dpu: add support for MDP_TOP blackhole
On 11/23/2022 1:04 PM, Dmitry Baryshkov wrote:
On sm8450 a register block was removed from MDP TOP. Accessing it during
snapshotting results in NoC errors / immediate reboot. Skip accessing
these registers during snapshot.
Tested-by: Vinod Koul
Reviewed-by: Vinod Koul
Reviewed-by: Konrad Dybcio
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 3 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 11 +--
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index 38aa38ab1568..8da4c5ba6dc3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -82,6 +82,8 @@ enum {
* @DPU_MDP_UBWC_1_0, This chipsets supports Universal Bandwidth
* compression initial revision
* @DPU_MDP_UBWC_1_5, Universal Bandwidth compression version 1.5
+ * @DPU_MDP_PERIPH_0_REMOVED Indicates that access to periph top0 block results
+ *in a failure
shouldnt this be that "indicates that the top register block is not
contiguous and the two sub-blocks are separated by an offset"
* @DPU_MDP_MAXMaximum value
*/
@@ -92,6 +94,7 @@ enum {
DPU_MDP_UBWC_1_0,
DPU_MDP_UBWC_1_5,
DPU_MDP_AUDIO_SELECT,
+ DPU_MDP_PERIPH_0_REMOVED,
DPU_MDP_MAX
};
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index f3660cd14f4f..4ac14de55139 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -927,8 +927,15 @@ static void dpu_kms_mdp_snapshot(struct msm_disp_state
*disp_state, struct msm_k
msm_disp_snapshot_add_block(disp_state, cat->wb[i].len,
dpu_kms->mmio + cat->wb[i].base, "wb_%d", i);
- msm_disp_snapshot_add_block(disp_state, cat->mdp[0].len,
- dpu_kms->mmio + cat->mdp[0].base, "top");
+ if (cat->mdp[0].features & BIT(DPU_MDP_PERIPH_0_REMOVED)) {
+ msm_disp_snapshot_add_block(disp_state, 0x380,
+ dpu_kms->mmio + cat->mdp[0].base, "top");
+ msm_disp_snapshot_add_block(disp_state, cat->mdp[0].len - 0x3a8,
+ dpu_kms->mmio + cat->mdp[0].base + 0x3a8,
"top_2");
I recall one of the comments from konrad that this should come from the
catalog rather than a hard-coded offset which you wanted to keep it for
a later time. I am fine with that.
But instead of a hard-coded offset, do you want to have a macro so that
atleast we know what the value means and can fix it in the future?
Otherwise it would end up being one of those numbers which someone later
on wouldnt understand where it comes from and what it means.
+ } else {
+ msm_disp_snapshot_add_block(disp_state, cat->mdp[0].len,
+ dpu_kms->mmio + cat->mdp[0].base, "top");
+ }
pm_runtime_put_sync(&dpu_kms->pdev->dev);
}
Re: [PATCH v6 2/7] platform/chrome: cros_ec_typec: Purge blocking switch devlinks
Hi Andy, Thanks for taking a look at this patch. Pin-Yen beat me to the punch with comment responses, but I'll add mine anyway. On Thu, Nov 24, 2022 at 4:25 AM Andy Shevchenko wrote: > ... > > > + /* > > + * OF graph may have set up some device links with switches, since > > connectors have their > > + * own compatible. Purge these to avoid a deadlock in switch probe > > (the switch mistakenly > > + * assumes the connector is a supplier). > > + */ > > A bit too long lines... They are within the 100 character limit [1] which is followed elsewhere in the driver; has something changed recently to make that invalid? > > > + if (dev->of_node) > > Why do you need this check? This issue only arises when using DT for this device. So the rationale is we don't need to perform this step on systems that don't use DT. Best regards, -Prashant [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144
Re: [PATCH v6 2/7] platform/chrome: cros_ec_typec: Purge blocking switch devlinks
Hi Andy, On Thu, Nov 24, 2022 at 8:25 PM Andy Shevchenko wrote: > > On Thu, Nov 24, 2022 at 06:20:51PM +0800, Pin-yen Lin wrote: > > From: Prashant Malani > > > > When using OF graph, the fw_devlink code will create links between the > > individual port driver (cros-ec-typec here) and the parent device for > > a Type-C switch (like mode-switch). Since the mode-switch will in turn > > have the usb-c-connector (i.e the child of the port driver) as a > > supplier, fw_devlink will not be able to resolve the cyclic dependency > > correctly. > > > > As a result, the mode-switch driver probe() never runs, so mode-switches > > are never registered. Because of that, the port driver probe constantly > > fails with -EPROBE_DEFER, because the Type-C connector class requires all > > switch devices to be registered prior to port registration. > > > > To break this deadlock and allow the mode-switch registration to occur, > > purge all the usb-c-connector nodes' absent suppliers. This eliminates > > the connector as a supplier for a switch and allows it to be probed. > > ... > > > + /* > > + * OF graph may have set up some device links with switches, since > > connectors have their > > + * own compatible. Purge these to avoid a deadlock in switch probe > > (the switch mistakenly > > + * assumes the connector is a supplier). > > + */ > > A bit too long lines... I'll fix this in v7. > > > + if (dev->of_node) > > Why do you need this check? We use this check to make sure only platforms using OF have their links purged. I'm not sure if this should also be done on x86 platforms. Best regards, Pin-yen > > > + device_for_each_child_node(dev, fwnode) > > + fw_devlink_purge_absent_suppliers(fwnode); > > -- > With Best Regards, > Andy Shevchenko > >
Re: [PATCH] drm/msm/mdp5: fix reading hw revision on db410c platform
On 11/24/2022 4:02 PM, Dmitry Baryshkov wrote:
Since the commit commit c6122688f265 ("drm/msm/mdp5: stop overriding
drvdata") reading the MDP5 hw revision on db410c will crash the board
as the MDSS_GDSC is not enabled. Revert a part of the offending commit
(moving rpm enablement) and set priv->kms earlier. This make it possible
to use pm_runtime_get_sync() during read_mdp_hw_revision(), which will
power up both the MDP5 and MDSS devices.
This is the exact concern I had even when c6122688f265 was pushed.
https://patchwork.freedesktop.org/patch/508334/#comment_917689
Was the response given that time not correct then?
https://patchwork.freedesktop.org/patch/508334/#comment_917713
Fixes: c6122688f265 ("drm/msm/mdp5: stop overriding drvdata")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index b46f983f2b46..29ae5c9613f3 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -519,10 +519,9 @@ static void read_mdp_hw_revision(struct mdp5_kms *mdp5_kms,
struct device *dev = &mdp5_kms->pdev->dev;
u32 version;
- /* Manually enable the MDP5, as pm runtime isn't usable yet. */
- mdp5_enable(mdp5_kms);
+ pm_runtime_get_sync(dev);
version = mdp5_read(mdp5_kms, REG_MDP5_HW_VERSION);
- mdp5_disable(mdp5_kms);
+ pm_runtime_put_sync(dev);
*major = FIELD(version, MDP5_HW_VERSION_MAJOR);
*minor = FIELD(version, MDP5_HW_VERSION_MINOR);
@@ -839,6 +838,12 @@ static int mdp5_init(struct platform_device *pdev, struct
drm_device *dev)
*/
clk_set_rate(mdp5_kms->core_clk, 2);
+ /* set uninit-ed kms */
+ priv->kms = &mdp5_kms->base.base;
+
+ pm_runtime_enable(&pdev->dev);
+ mdp5_kms->rpm_enabled = true;
+
read_mdp_hw_revision(mdp5_kms, &major, &minor);
mdp5_kms->cfg = mdp5_cfg_init(mdp5_kms, major, minor);
@@ -887,12 +892,6 @@ static int mdp5_init(struct platform_device *pdev, struct
drm_device *dev)
if (ret)
goto fail;
- /* set uninit-ed kms */
- priv->kms = &mdp5_kms->base.base;
-
- pm_runtime_enable(&pdev->dev);
- mdp5_kms->rpm_enabled = true;
-
return 0;
fail:
if (mdp5_kms)
Re: [PATCH v6 6/7] dt/bindings: drm/bridge: it6505: Add mode-switch support
Sorry for accidentally using the tab characters. Will fix this in v7. On Fri, Nov 25, 2022 at 1:39 AM Rob Herring wrote: > > > On Thu, 24 Nov 2022 18:20:55 +0800, Pin-yen Lin wrote: > > ITE IT6505 can be used in systems to switch the DP traffic between > > two downstreams, which can be USB Type-C DisplayPort alternate mode > > lane or regular DisplayPort output ports. > > > > Update the binding to accommodate this usage by introducing a > > data-lanes and a mode-switch property on endpoints. > > > > Signed-off-by: Pin-yen Lin > > > > --- > > > > Changes in v6: > > - Remove switches node and use endpoints and data-lanes property to > > describe the connections. > > > > .../bindings/display/bridge/ite,it6505.yaml | 94 ++- > > 1 file changed, 90 insertions(+), 4 deletions(-) > > > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > yamllint warnings/errors: > ./Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml:99:1: > [error] syntax error: found character '\t' that cannot start any token > (syntax) > > dtschema/dtc warnings/errors: > make[1]: *** Deleting file > 'Documentation/devicetree/bindings/display/bridge/ite,it6505.example.dts' > Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml:99:1: found > character '\t' that cannot start any token > make[1]: *** [Documentation/devicetree/bindings/Makefile:26: > Documentation/devicetree/bindings/display/bridge/ite,it6505.example.dts] > Error 1 > make[1]: *** Waiting for unfinished jobs > ./Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml:99:1: > found character '\t' that cannot start any token > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml: > ignoring, error parsing file > make: *** [Makefile:1492: dt_binding_check] Error 2 > > doc reference errors (make refcheckdocs): > > See > https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221124102056.393220-7-treapk...@chromium.org > > This check can fail if there are any dependencies. The base for a patch > series is generally the most recent rc1. > > If you already ran 'make dt_binding_check' and didn't see the above > error(s), then make sure 'yamllint' is installed and dt-schema is up to > date: > > pip3 install dtschema --upgrade > > Please check and re-submit after running the above command. >
Re: [PATCH v6 3/7] dt-bindings: drm/bridge: anx7625: Add mode-switch support
Sorry for accidentally using the tab characters. Will fix this in v7. On Fri, Nov 25, 2022 at 1:39 AM Rob Herring wrote: > > > On Thu, 24 Nov 2022 18:20:52 +0800, Pin-yen Lin wrote: > > Analogix 7625 can be used in systems to switch the DP traffic between > > two downstreams, which can be USB Type-C DisplayPort alternate mode > > lane or regular DisplayPort output ports. > > > > Update the binding to accommodate this usage by introducing a > > data-lanes and a mode-switch property on endpoints. > > > > Also include the link to the product brief in the bindings. > > > > Signed-off-by: Pin-yen Lin > > > > --- > > > > Changes in v6: > > - Remove switches node and use endpoints and data-lanes property to > > describe the connections. > > > > .../display/bridge/analogix,anx7625.yaml | 73 ++- > > 1 file changed, 71 insertions(+), 2 deletions(-) > > > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > yamllint warnings/errors: > ./Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml:218:1: > [error] syntax error: found character '\t' that cannot start any token > (syntax) > > dtschema/dtc warnings/errors: > make[1]: *** Deleting file > 'Documentation/devicetree/bindings/display/bridge/analogix,anx7625.example.dts' > Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml:218:1: > found character '\t' that cannot start any token > make[1]: *** [Documentation/devicetree/bindings/Makefile:26: > Documentation/devicetree/bindings/display/bridge/analogix,anx7625.example.dts] > Error 1 > make[1]: *** Waiting for unfinished jobs > ./Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml:218:1: > found character '\t' that cannot start any token > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml: > ignoring, error parsing file > make: *** [Makefile:1492: dt_binding_check] Error 2 > > doc reference errors (make refcheckdocs): > > See > https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221124102056.393220-4-treapk...@chromium.org > > This check can fail if there are any dependencies. The base for a patch > series is generally the most recent rc1. > > If you already ran 'make dt_binding_check' and didn't see the above > error(s), then make sure 'yamllint' is installed and dt-schema is up to > date: > > pip3 install dtschema --upgrade > > Please check and re-submit after running the above command. >
[git pull] drm fixes for 6.1-rc7
Hi Linus, Weekly fixes, amdgpu has not quite settled down. Most of the changes are small, and the non-amdgpu ones are all fine. There are a bunch of DP MST DSC fixes that fix some issues introduced in a previous larger MST rework. The biggest one is mainly propagating some error values properly instead of bool returns, and I think it just looks large but doesn't really change anything too much, except propagating errors that are required to avoid deadlocks. I've gone over it and a few others and they've had some decent testing over the last few weeks, but let me know if you think it's too problematic. Dave. drm-fixes-2022-11-25: drm fixes for 6.1-rc7 amdgpu: - amdgpu gang submit fix. - DCN 3.1.4 fixes - DP MST DSC deadlock fixes - HMM userptr fixes - Fix Aldebaran CU occupancy reporting - GFX11 fixes - PSP suspend/resume fix - DCE12 KASAN fix - DCN 3.2.x fixes - Rotated cursor fix - SMU 13.x fix - DELL platform suspend/resume fixes - VCN4 SR-IOV fix - Display regression fix for polled connectors i915: - Fix GVT KVM reference count handling - Never purge busy TTM objects - Fix warn in intel_display_power_*_domain() functions dma-buf: - Use dma_fence_unwrap_for_each when importing sync files. - Fix race in dma_heap_add(). fbcon: - Fix use of uninitialized memory in logo. The following changes since commit eb7081409f94a9a8608593d0fb63a1aa3d6f95d8: Linux 6.1-rc6 (2022-11-20 16:02:16 -0800) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2022-11-25 for you to fetch changes up to e57702069b26b8601a33fdc0c9bbe40c6bb9c72f: Merge tag 'amd-drm-fixes-6.1-2022-11-23' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes (2022-11-25 10:55:23 +1000) drm fixes for 6.1-rc7 amdgpu: - amdgpu gang submit fix. - DCN 3.1.4 fixes - DP MST DSC deadlock fixes - HMM userptr fixes - Fix Aldebaran CU occupancy reporting - GFX11 fixes - PSP suspend/resume fix - DCE12 KASAN fix - DCN 3.2.x fixes - Rotated cursor fix - SMU 13.x fix - DELL platform suspend/resume fixes - VCN4 SR-IOV fix - Display regression fix for polled connectors i915: - Fix GVT KVM reference count handling - Never purge busy TTM objects - Fix warn in intel_display_power_*_domain() functions dma-buf: - Use dma_fence_unwrap_for_each when importing sync files. - Fix race in dma_heap_add(). fbcon: - Fix use of uninitialized memory in logo. Alex Deucher (2): drm/amdgpu/psp: don't free PSP buffers on suspend drm/amdgpu: Partially revert "drm/amdgpu: update drm_display_info correctly when the edid is read" Christian König (3): drm/amdgpu: handle gang submit before VMID drm/amdgpu: always register an MMU notifier for userptr drm/amdgpu: fix userptr HMM range handling v2 Dave Airlie (3): Merge tag 'drm-misc-fixes-2022-11-24' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes Merge tag 'drm-intel-fixes-2022-11-24' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes Merge tag 'amd-drm-fixes-6.1-2022-11-23' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes David Galiffi (1): drm/amd/display: Fix rotated cursor offset calculation Dawei Li (1): dma-buf: fix racing conflict of dma_heap_add() Dillon Varone (3): drm/amd/display: Update soc bounding box for dcn32/dcn321 drm/amd/display: Use viewport height for subvp mall allocation size drm/amd/display: Use new num clk levels struct for max mclk index Imre Deak (1): drm/i915: Fix warn in intel_display_power_*_domain() functions Jack Xiao (1): drm/amd/amdgpu: reserve vm invalidation engine for firmware Jane Jian (1): drm/amdgpu/vcn: re-use original vcn0 doorbell value Jason Ekstrand (1): dma-buf: Use dma_fence_unwrap_for_each when importing fences Lyude Paul (6): drm/amdgpu/mst: Stop ignoring error codes and deadlocking drm/display/dp_mst: Fix drm_dp_mst_add_affected_dsc_crtcs() return code drm/amdgpu/dm/mst: Use the correct topology mgr pointer in amdgpu_dm_connector drm/amdgpu/dm/dp_mst: Don't grab mst_mgr->lock when computing DSC state drm/amdgpu/dm/mst: Fix uninitialized var in pre_compute_mst_dsc_configs_for_state() drm/amd/dc/dce120: Fix audio register mapping, stop triggering KASAN Matthew Auld (1): drm/i915/ttm: never purge busy objects Ramesh Errabolu (1): drm/amdgpu: Enable Aldebaran devices to report CU Occupancy Roman Li (1): drm/amd/display: Align dcn314_smu logging with other DCNs Sean Christopherson (2): drm/i915/gvt: Get reference to KVM iff attachment to VM is successful drm/i915/gvt: Unconditionally put reference to KVM when detaching vGPU Stanley.Yang (1): drm/amdgpu: fix use-after-free during gpu recovery Taimur Hassan (1): drm/amd/display: Avoid setting pixel rate divider to N/A Tetsuo
Re: [PATCH] swsmu/amdgpu_smu: Fix the wrong if-condition
Applied!
Thanks,
Luben
On 2022-11-24 05:10, Quan, Evan wrote:
> [AMD Official Use Only - General]
>
> Reviewed-by: Evan Quan
>
>> -Original Message-
>> From: amd-gfx On Behalf Of Yu
>> Songping
>> Sent: Thursday, November 24, 2022 9:53 AM
>> To: airl...@gmail.com; dan...@ffwll.ch
>> Cc: dri-devel@lists.freedesktop.org; amd-...@lists.freedesktop.org; linux-
>> ker...@vger.kernel.org
>> Subject: [PATCH] swsmu/amdgpu_smu: Fix the wrong if-condition
>>
>> The logical operator '&&' will make
>> smu->ppt_funcs->set_gfx_power_up_by_imu segment fault when
>> ppt_funcs is
>> smu->NULL.
>>
>> Signed-off-by: Yu Songping
>> ---
>> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> index b880f4d7d67e..1cb728b0b7ee 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> @@ -161,7 +161,7 @@ int smu_get_dpm_freq_range(struct smu_context
>> *smu,
>>
>> int smu_set_gfx_power_up_by_imu(struct smu_context *smu) {
>> - if (!smu->ppt_funcs && !smu->ppt_funcs-
>>> set_gfx_power_up_by_imu)
>> + if (!smu->ppt_funcs || !smu->ppt_funcs-
>>> set_gfx_power_up_by_imu)
>> return -EOPNOTSUPP;
>>
>> return smu->ppt_funcs->set_gfx_power_up_by_imu(smu);
>> --
>> 2.17.1
RE: [PATCH] drm/amdgpu: add mb for si
[AMD Official Use Only - General] > -Original Message- > From: Lazar, Lijo > Sent: Thursday, November 24, 2022 6:49 PM > To: Quan, Evan ; 李真能 ; > Michel Dänzer ; Koenig, Christian > ; Deucher, Alexander > > Cc: amd-...@lists.freedesktop.org; Pan, Xinhui ; > linux-ker...@vger.kernel.org; dri-devel@lists.freedesktop.org > Subject: Re: [PATCH] drm/amdgpu: add mb for si > > > > On 11/24/2022 4:11 PM, Lazar, Lijo wrote: > > > > > > On 11/24/2022 3:34 PM, Quan, Evan wrote: > >> [AMD Official Use Only - General] > >> > >> Could the attached patch help? > >> > >> Evan > >>> -Original Message- > >>> From: amd-gfx On Behalf > Of ??? > >>> Sent: Friday, November 18, 2022 5:25 PM > >>> To: Michel Dänzer ; Koenig, Christian > >>> ; Deucher, Alexander > >>> > >>> Cc: amd-...@lists.freedesktop.org; Pan, Xinhui ; > >>> linux-ker...@vger.kernel.org; dri-devel@lists.freedesktop.org > >>> Subject: Re: [PATCH] drm/amdgpu: add mb for si > >>> > >>> > >>> 在 2022/11/18 17:18, Michel Dänzer 写道: > On 11/18/22 09:01, Christian König wrote: > > Am 18.11.22 um 08:48 schrieb Zhenneng Li: > >> During reboot test on arm64 platform, it may failure on boot, so > >> add this mb in smc. > >> > >> The error message are as follows: > >> [ 6.996395][ 7] [ T295] [drm:amdgpu_device_ip_late_init > >> [amdgpu]] *ERROR* > >> late_init of IP block failed -22 [ > >> 7.006919][ 7] [ T295] amdgpu :04:00.0: > > > > The issue is happening in late_init() which eventually does > > > > ret = si_thermal_enable_alert(adev, false); > > > > Just before this, si_thermal_start_thermal_controller is called in > > hw_init and that enables thermal alert. > > > > Maybe the issue is with enable/disable of thermal alerts in quick > > succession. Adding a delay inside si_thermal_start_thermal_controller > > might help. > > > > On a second look, temperature range is already set as part of > si_thermal_start_thermal_controller in hw_init > https://elixir.bootlin.com/linux/v6.1- > rc6/source/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c#L6780 > > There is no need to set it again here - > > https://elixir.bootlin.com/linux/v6.1- > rc6/source/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c#L7635 > > I think it is safe to remove the call from late_init altogether. Alex/Evan? > [Quan, Evan] Yes, it makes sense to me. But I'm not sure whether that’s related with the issue here. Since per my understandings, if the issue is caused by double calling of thermal_alert enablement, it will fail every time. That cannot explain why adding some delays or a mb() calling can help. BR Evan > Thanks, > Lijo > > > Thanks, > > Lijo > > > >> amdgpu_device_ip_late_init failed [ 7.014224][ 7] [ T295] amdgpu > >> :04:00.0: Fatal error during GPU init > > Memory barries are not supposed to be sprinkled around like this, > you > >>> need to give a detailed explanation why this is necessary. > > > > Regards, > > Christian. > > > >> Signed-off-by: Zhenneng Li > >> --- > >> drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c > >> b/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c > >> index 8f994ffa9cd1..c7656f22278d 100644 > >> --- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c > >> +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c > >> @@ -155,6 +155,8 @@ bool amdgpu_si_is_smc_running(struct > >> amdgpu_device *adev) > >> u32 rst = RREG32_SMC(SMC_SYSCON_RESET_CNTL); > >> u32 clk = RREG32_SMC(SMC_SYSCON_CLOCK_CNTL_0); > >> + mb(); > >> + > >> if (!(rst & RST_REG) && !(clk & CK_DISABLE)) > >> return true; > In particular, it makes no sense in this specific place, since it > cannot directly > >>> affect the values of rst & clk. > >>> > >>> I thinks so too. > >>> > >>> But when I do reboot test using nine desktop machines, there maybe > >>> report > >>> this error on one or two machines after Hundreds of times or > >>> Thousands of > >>> times reboot test, at the beginning, I use msleep() instead of mb(), > >>> these > >>> two methods are all works, but I don't know what is the root case. > >>> > >>> I use this method on other verdor's oland card, this error message are > >>> reported again. > >>> > >>> What could be the root reason? > >>> > >>> test environmen: > >>> > >>> graphics card: OLAND 0x1002:0x6611 0x1642:0x1869 0x87 > >>> > >>> driver: amdgpu > >>> > >>> os: ubuntu 2004 > >>> > >>> platform: arm64 > >>> > >>> kernel: 5.4.18 > >>> > <>
RE: [PATCH] drm/amdgpu: add mb for si
[AMD Official Use Only - General]
Did you see that? It's a patch which I created by git-format-patch.
Anyway I will paste the changes below. I was suspecting maybe we need some
waits for smu running.
diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
index 49c398ec0aaf..9f308a021b2d 100644
--- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
+++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
@@ -6814,6 +6814,7 @@ static int si_dpm_enable(struct amdgpu_device *adev)
struct si_power_info *si_pi = si_get_pi(adev);
struct amdgpu_ps *boot_ps = adev->pm.dpm.boot_ps;
int ret;
+ int i;
if (amdgpu_si_is_smc_running(adev))
return -EINVAL;
@@ -6909,6 +6910,17 @@ static int si_dpm_enable(struct amdgpu_device *adev)
si_program_response_times(adev);
si_program_ds_registers(adev);
si_dpm_start_smc(adev);
+ /* Waiting for smc alive */
+ for (i = 0; i < adev->usec_timeout; i++) {
+ if (amdgpu_si_is_smc_running(adev))
+ break;
+ udelay(1);
+ }
+ if (i >= adev->usec_timeout) {
+ DRM_ERROR("Timedout on waiting for smu running\n");
+ return -EINVAL;
+ }
+
ret = si_notify_smc_display_change(adev, false);
if (ret) {
DRM_ERROR("si_notify_smc_display_change failed\n");
BR
Evan
> -Original Message-
> From: Christian König
> Sent: Thursday, November 24, 2022 6:06 PM
> To: Quan, Evan ; 李真能 ;
> Michel Dänzer ; Koenig, Christian
> ; Deucher, Alexander
>
> Cc: dri-devel@lists.freedesktop.org; Pan, Xinhui ;
> linux-ker...@vger.kernel.org; amd-...@lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu: add mb for si
>
> That's not a patch but some binary file?
>
> Christian.
>
> Am 24.11.22 um 11:04 schrieb Quan, Evan:
> > [AMD Official Use Only - General]
> >
> > Could the attached patch help?
> >
> > Evan
> >> -Original Message-
> >> From: amd-gfx On Behalf
> Of ???
> >> Sent: Friday, November 18, 2022 5:25 PM
> >> To: Michel Dänzer ; Koenig, Christian
> >> ; Deucher, Alexander
> >>
> >> Cc: amd-...@lists.freedesktop.org; Pan, Xinhui ;
> >> linux-ker...@vger.kernel.org; dri-devel@lists.freedesktop.org
> >> Subject: Re: [PATCH] drm/amdgpu: add mb for si
> >>
> >>
> >> 在 2022/11/18 17:18, Michel Dänzer 写道:
> >>> On 11/18/22 09:01, Christian König wrote:
> Am 18.11.22 um 08:48 schrieb Zhenneng Li:
> > During reboot test on arm64 platform, it may failure on boot, so
> > add this mb in smc.
> >
> > The error message are as follows:
> > [ 6.996395][ 7] [ T295] [drm:amdgpu_device_ip_late_init
> > [amdgpu]] *ERROR*
> > late_init of IP block failed -22 [
> > 7.006919][ 7] [ T295] amdgpu :04:00.0:
> > amdgpu_device_ip_late_init failed [ 7.014224][ 7] [ T295]
> > amdgpu
> > :04:00.0: Fatal error during GPU init
> Memory barries are not supposed to be sprinkled around like this,
> you
> >> need to give a detailed explanation why this is necessary.
> Regards,
> Christian.
>
> > Signed-off-by: Zhenneng Li
> > ---
> > drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c
> > b/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c
> > index 8f994ffa9cd1..c7656f22278d 100644
> > --- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c
> > +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c
> > @@ -155,6 +155,8 @@ bool amdgpu_si_is_smc_running(struct
> > amdgpu_device *adev)
> > u32 rst = RREG32_SMC(SMC_SYSCON_RESET_CNTL);
> > u32 clk = RREG32_SMC(SMC_SYSCON_CLOCK_CNTL_0);
> > + mb();
> > +
> > if (!(rst & RST_REG) && !(clk & CK_DISABLE))
> > return true;
> >>> In particular, it makes no sense in this specific place, since it
> >>> cannot directly
> >> affect the values of rst & clk.
> >>
> >> I thinks so too.
> >>
> >> But when I do reboot test using nine desktop machines, there maybe
> >> report this error on one or two machines after Hundreds of times or
> >> Thousands of times reboot test, at the beginning, I use msleep()
> >> instead of mb(), these two methods are all works, but I don't know what
> is the root case.
> >>
> >> I use this method on other verdor's oland card, this error message
> >> are reported again.
> >>
> >> What could be the root reason?
> >>
> >> test environmen:
> >>
> >> graphics card: OLAND 0x1002:0x6611 0x1642:0x1869 0x87
> >>
> >> driver: amdgpu
> >>
> >> os: ubuntu 2004
> >>
> >> platform: arm64
> >>
> >> kernel: 5.4.18
> >>
<>
[PATCH] drm/mediatek: Add checks for devm_kcalloc
As the devm_kcalloc may return NULL, the return value needs to be checked
to avoid NULL poineter dereference.
Fixes: 66b2cf9623fa ("drm/mediatek: use layer_nr function to get layer number
to init plane")
Signed-off-by: Yuan Can
---
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 112615817dcb..5071f1263216 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -945,6 +945,8 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
mtk_crtc->planes = devm_kcalloc(dev, num_comp_planes,
sizeof(struct drm_plane), GFP_KERNEL);
+ if (!mtk_crtc->planes)
+ return -ENOMEM;
for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
ret = mtk_drm_crtc_init_comp_planes(drm_dev, mtk_crtc, i,
--
2.17.1
[PATCH] drm/msm/mdp5: fix reading hw revision on db410c platform
Since the commit commit c6122688f265 ("drm/msm/mdp5: stop overriding
drvdata") reading the MDP5 hw revision on db410c will crash the board
as the MDSS_GDSC is not enabled. Revert a part of the offending commit
(moving rpm enablement) and set priv->kms earlier. This make it possible
to use pm_runtime_get_sync() during read_mdp_hw_revision(), which will
power up both the MDP5 and MDSS devices.
Fixes: c6122688f265 ("drm/msm/mdp5: stop overriding drvdata")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index b46f983f2b46..29ae5c9613f3 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -519,10 +519,9 @@ static void read_mdp_hw_revision(struct mdp5_kms *mdp5_kms,
struct device *dev = &mdp5_kms->pdev->dev;
u32 version;
- /* Manually enable the MDP5, as pm runtime isn't usable yet. */
- mdp5_enable(mdp5_kms);
+ pm_runtime_get_sync(dev);
version = mdp5_read(mdp5_kms, REG_MDP5_HW_VERSION);
- mdp5_disable(mdp5_kms);
+ pm_runtime_put_sync(dev);
*major = FIELD(version, MDP5_HW_VERSION_MAJOR);
*minor = FIELD(version, MDP5_HW_VERSION_MINOR);
@@ -839,6 +838,12 @@ static int mdp5_init(struct platform_device *pdev, struct
drm_device *dev)
*/
clk_set_rate(mdp5_kms->core_clk, 2);
+ /* set uninit-ed kms */
+ priv->kms = &mdp5_kms->base.base;
+
+ pm_runtime_enable(&pdev->dev);
+ mdp5_kms->rpm_enabled = true;
+
read_mdp_hw_revision(mdp5_kms, &major, &minor);
mdp5_kms->cfg = mdp5_cfg_init(mdp5_kms, major, minor);
@@ -887,12 +892,6 @@ static int mdp5_init(struct platform_device *pdev, struct
drm_device *dev)
if (ret)
goto fail;
- /* set uninit-ed kms */
- priv->kms = &mdp5_kms->base.base;
-
- pm_runtime_enable(&pdev->dev);
- mdp5_kms->rpm_enabled = true;
-
return 0;
fail:
if (mdp5_kms)
--
2.35.1
Re: [PATCH] drm/amdgpu: remove redundant NULL check
Applied.
Thanks,
Luben
On 2022-11-23 02:44, zys.zlj...@gmail.com wrote:
> From: Yushan Zhou
>
> release_firmware() checks whether firmware pointer is NULL.
> Remove the redundant NULL check in psp_sw_fini().
>
> Signed-off-by: Yushan Zhou
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 36 +++--
> 1 file changed, 16 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index effa7df3ddbf..77b966ab5439 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -484,26 +484,22 @@ static int psp_sw_fini(void *handle)
> struct psp_gfx_cmd_resp *cmd = psp->cmd;
>
> psp_memory_training_fini(psp);
> - if (psp->sos_fw) {
> - release_firmware(psp->sos_fw);
> - psp->sos_fw = NULL;
> - }
> - if (psp->asd_fw) {
> - release_firmware(psp->asd_fw);
> - psp->asd_fw = NULL;
> - }
> - if (psp->ta_fw) {
> - release_firmware(psp->ta_fw);
> - psp->ta_fw = NULL;
> - }
> - if (psp->cap_fw) {
> - release_firmware(psp->cap_fw);
> - psp->cap_fw = NULL;
> - }
> - if (psp->toc_fw) {
> - release_firmware(psp->toc_fw);
> - psp->toc_fw = NULL;
> - }
> +
> + release_firmware(psp->sos_fw);
> + psp->sos_fw = NULL;
> +
> + release_firmware(psp->asd_fw);
> + psp->asd_fw = NULL;
> +
> + release_firmware(psp->ta_fw);
> + psp->ta_fw = NULL;
> +
> + release_firmware(psp->cap_fw);
> + psp->cap_fw = NULL;
> +
> + release_firmware(psp->toc_fw);
> + psp->toc_fw = NULL;
> +
> if (adev->ip_versions[MP0_HWIP][0] == IP_VERSION(11, 0, 0) ||
> adev->ip_versions[MP0_HWIP][0] == IP_VERSION(11, 0, 7))
> psp_sysfs_fini(adev);
Re: [PATCH] drm/dma-helpers: Don't change vma flags
On Thu, Nov 24, 2022 at 06:24:52PM +0100, Daniel Vetter wrote:
> On Thu, Nov 24, 2022 at 11:11:21AM +, Robin Murphy wrote:
> > On 2022-11-23 17:28, Daniel Vetter wrote:
> > > This code was added in b65e64f7ccd4 ("drm/cma: Use
> > > dma_mmap_writecombine() to mmap buffer"), but does not explain why
> > > it's needed.
> > >
> > > It should be entirely unnecessary, because remap_pfn_range(), which is
> > > what the various dma_mmap functiosn are built on top of, does already
> > > unconditionally adjust the vma flags:
> >
> > Not all dma_mmap_*() implementations use remap_pfn_range() though. For
> > instance on ARM where one set of DMA ops uses vm_map_pages(), but AFAICS not
> > all the relevant drivers would set VM_MIXEDMAP to prevent reaching the
> > BUG_ON(vma->vm_flags & VM_PFNMAP) in there.
>
> Uh a dma_mmap which does not use VM_PFNMAP has pretty good chances of
> being busted, since that allows get_user_pages on these memory
> allocations. And I'm really not sure that's a bright idea ...
>
> Can you please point me at these dma-ops so that I can try and understand
> what they're trying to do?
I've taken a trip back in time to v3.14, when b65e64f7ccd4 was merged,
and checked the x86, arm and arm64 dma mapping implementations. They all
call remap_pfn_range() except for arm_iommu_mmap_attrs(), which uses
vm_insert_page(). It was implemented as
int vm_insert_page(struct vm_area_struct *vma, unsigned long addr,
struct page *page)
{
if (addr < vma->vm_start || addr >= vma->vm_end)
return -EFAULT;
if (!page_count(page))
return -EINVAL;
if (!(vma->vm_flags & VM_MIXEDMAP)) {
BUG_ON(down_read_trylock(&vma->vm_mm->mmap_sem));
BUG_ON(vma->vm_flags & VM_PFNMAP);
vma->vm_flags |= VM_MIXEDMAP;
}
return insert_page(vma, addr, page, vma->vm_page_prot);
}
I don't recall if I just hit that BUG_ON() back then, or if I actually
understood what I was doing :-)
Today in mainline arm_iommu_mmap_attrs() uses vm_map_pages(), which
calls vm_insert_page(), and the BUG_ON() is still there.
> > > https://elixir.bootlin.com/linux/v6.1-rc6/source/mm/memory.c#L2518
> > >
> > > More importantly, it does uncondtionally set VM_PFNMAP, so clearing
> > > that does not make much sense.
> > >
> > > Patch motived by discussions around enforcing VM_PFNMAP semantics for
> > > all dma-buf users, where Thomas asked why dma helpers will work with
> > > that dma_buf_mmap() contract.
> > >
> > > References:
> > > https://lore.kernel.org/dri-devel/5c3c8d4f-2c06-9210-b00a-4d0ff6f6f...@suse.de/
> > > Cc: Laurent Pinchart
> > > Cc: Dave Airlie
> > > Cc: Maarten Lankhorst
> > > Cc: Maxime Ripard
> > > Cc: Thomas Zimmermann
> > > Cc: Sumit Semwal
> > > Cc: "Christian König"
> > > Signed-off-by: Daniel Vetter
> > > ---
> > > drivers/gpu/drm/drm_gem_dma_helper.c | 7 ++-
> > > 1 file changed, 2 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_gem_dma_helper.c
> > > b/drivers/gpu/drm/drm_gem_dma_helper.c
> > > index 1e658c448366..637a5cc62457 100644
> > > --- a/drivers/gpu/drm/drm_gem_dma_helper.c
> > > +++ b/drivers/gpu/drm/drm_gem_dma_helper.c
> > > @@ -525,13 +525,10 @@ int drm_gem_dma_mmap(struct drm_gem_dma_object
> > > *dma_obj, struct vm_area_struct *
> > > int ret;
> > > /*
> > > - * Clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and set the
> > > - * vm_pgoff (used as a fake buffer offset by DRM) to 0 as we want to map
> > > - * the whole buffer.
> > > + * Set the vm_pgoff (used as a fake buffer offset by DRM) to 0 as we
> > > + * want to map the whole buffer.
> > >*/
> > > vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node);
> > > - vma->vm_flags &= ~VM_PFNMAP;
> > > - vma->vm_flags |= VM_DONTEXPAND;
> > > if (dma_obj->map_noncoherent) {
> > > vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
--
Regards,
Laurent Pinchart
[linux-next:master] BUILD REGRESSION c35bd4e428856ed8c1fae7f7dfa08a9141c153d1
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: c35bd4e428856ed8c1fae7f7dfa08a9141c153d1 Add linux-next specific files for 20221124 Error/Warning reports: https://lore.kernel.org/oe-kbuild-all/202211090634.ryfkk0ws-...@intel.com https://lore.kernel.org/oe-kbuild-all/202211241736.k6437e7j-...@intel.com https://lore.kernel.org/oe-kbuild-all/202211242021.fdzrfna8-...@intel.com https://lore.kernel.org/oe-kbuild-all/202211242120.mzzvguln-...@intel.com https://lore.kernel.org/oe-kbuild-all/202211250227.grpjpxyn-...@intel.com Error/Warning: (recently discovered and may have been fixed) arch/arm/mach-s3c/devs.c:32:10: fatal error: linux/platform_data/dma-s3c24xx.h: No such file or directory arch/powerpc/kernel/kvm_emul.o: warning: objtool: kvm_template_end(): can't find starting instruction arch/powerpc/kernel/optprobes_head.o: warning: objtool: optprobe_template_end(): can't find starting instruction drivers/clk/clk.c:1022:5: error: redefinition of 'clk_prepare' drivers/clk/clk.c:1268:6: error: redefinition of 'clk_is_enabled_when_prepared' drivers/clk/clk.c:941:6: error: redefinition of 'clk_unprepare' drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:4968: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:5075:24: warning: implicit conversion from 'enum ' to 'enum dc_status' [-Wenum-conversion] drivers/gpu/drm/amd/amdgpu/../display/dc/irq/dcn201/irq_service_dcn201.c:40:20: warning: no previous prototype for 'to_dal_irq_source_dcn201' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/fifo/gf100.c:451:1: warning: no previous prototype for 'gf100_fifo_nonstall_block' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/fifo/gf100.c:451:1: warning: no previous prototype for function 'gf100_fifo_nonstall_block' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/fifo/runl.c:34:1: warning: no previous prototype for 'nvkm_engn_cgrp_get' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/fifo/runl.c:34:1: warning: no previous prototype for function 'nvkm_engn_cgrp_get' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/gr/tu102.c:210:1: warning: no previous prototype for 'tu102_gr_load' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/gr/tu102.c:210:1: warning: no previous prototype for function 'tu102_gr_load' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/nvfw/acr.c:49:1: warning: no previous prototype for 'wpr_generic_header_dump' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/nvfw/acr.c:49:1: warning: no previous prototype for function 'wpr_generic_header_dump' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c:221:21: warning: variable 'loc' set but not used [-Wunused-but-set-variable] drivers/iio/addac/ad74115.c:320:27: warning: 'ad74115_dac_slew_rate_hz_tbl' defined but not used [-Wunused-const-variable=] mm/vmscan.c:4090:30: error: implicit declaration of function 'pmd_young'; did you mean 'pte_young'? [-Werror=implicit-function-declaration] net/netfilter/nf_conntrack_netlink.c:2674:6: warning: unused variable 'mark' [-Wunused-variable] vmlinux.o: warning: objtool: __btrfs_map_block+0x1d77: unreachable instruction Unverified Error/Warning (likely false positive, please contact us if interested): drivers/usb/fotg210/fotg210-udc.c:632:17: sparse: sparse: restricted __le16 degrades to integer Error/Warning ids grouped by kconfigs: gcc_recent_errors |-- alpha-allyesconfig | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc.c:warning:This-comment-starts-with-but-isn-t-a-kernel-doc-comment.-Refer-Documentation-doc-guide-kernel-doc.rst | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc_link_dp.c:warning:implicit-conversion-from-enum-anonymous-to-enum-dc_status | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-irq-dcn201-irq_service_dcn201.c:warning:no-previous-prototype-for-to_dal_irq_source_dcn201 | |-- drivers-gpu-drm-nouveau-nvkm-engine-fifo-gf100.c:warning:no-previous-prototype-for-gf100_fifo_nonstall_block | |-- drivers-gpu-drm-nouveau-nvkm-engine-fifo-runl.c:warning:no-previous-prototype-for-nvkm_engn_cgrp_get | |-- drivers-gpu-drm-nouveau-nvkm-engine-gr-tu102.c:warning:no-previous-prototype-for-tu102_gr_load | |-- drivers-gpu-drm-nouveau-nvkm-nvfw-acr.c:warning:no-previous-prototype-for-wpr_generic_header_dump | `-- drivers-gpu-drm-nouveau-nvkm-subdev-acr-lsfw.c:warning:variable-loc-set-but-not-used |-- alpha-randconfig-r005-20221124 | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc.c:warning:This-comment-starts-with-but-isn-t-a-kernel-doc-comment.-Refer-Documentation-doc-
Re: [PATCH v4 0/4] new subsystem for compute accelerator devices
Hi Oded, On Sat, 19 Nov 2022 at 20:44, Oded Gabbay wrote: > This is the fourth (and hopefully last) version of the patch-set to add the > new subsystem for compute accelerators. I removed the RFC headline as > I believe it is now ready for merging. > > Compare to v3, this patch-set contains one additional patch that adds > documentation regarding the accel subsystem. I hope it's good enough for > this stage. In addition, there were few very minor fixes according to > comments received on v3. > > The patches are in the following repo: > https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git/log/?h=accel_v4 Series is: Acked-by: Daniel Stone Cheers, Daniel
Re: [PATCH] drm/dma-helpers: Don't change vma flags
On 2022-11-24 17:24, Daniel Vetter wrote:
On Thu, Nov 24, 2022 at 11:11:21AM +, Robin Murphy wrote:
On 2022-11-23 17:28, Daniel Vetter wrote:
This code was added in b65e64f7ccd4 ("drm/cma: Use
dma_mmap_writecombine() to mmap buffer"), but does not explain why
it's needed.
It should be entirely unnecessary, because remap_pfn_range(), which is
what the various dma_mmap functiosn are built on top of, does already
unconditionally adjust the vma flags:
Not all dma_mmap_*() implementations use remap_pfn_range() though. For
instance on ARM where one set of DMA ops uses vm_map_pages(), but AFAICS not
all the relevant drivers would set VM_MIXEDMAP to prevent reaching the
BUG_ON(vma->vm_flags & VM_PFNMAP) in there.
Uh a dma_mmap which does not use VM_PFNMAP has pretty good chances of
being busted, since that allows get_user_pages on these memory
allocations. And I'm really not sure that's a bright idea ...
Can you please point me at these dma-ops so that I can try and understand
what they're trying to do?
See arm_iommu_mmap_attrs(), but also one of the paths in
iommu_dma_mmap() for both arm64 and x86. These aren't using
remap_pfn_range() because they're mapping a potentially-disjoint set of
arbitrary pages, not a single physically-contiguous range. And for the
avoidance of doubt, yes, in those cases they will always be real kernel
pages. dma_mmap_attrs() can be relied upon to do the right thing for
whatever dma_alloc_attrs() did; what isn't reliable is trying to
second-guess from outside exactly what that might be.
I forgot to mention also that removing the VM_DONTEXPAND line will
seemingly just reintroduce the annoying warning spam for which we added
it in the first place (and 59f39bfa6553 does document this same reasoning).
Thanks,
Robin.
-Daniel
Robin.
https://elixir.bootlin.com/linux/v6.1-rc6/source/mm/memory.c#L2518
More importantly, it does uncondtionally set VM_PFNMAP, so clearing
that does not make much sense.
Patch motived by discussions around enforcing VM_PFNMAP semantics for
all dma-buf users, where Thomas asked why dma helpers will work with
that dma_buf_mmap() contract.
References:
https://lore.kernel.org/dri-devel/5c3c8d4f-2c06-9210-b00a-4d0ff6f6f...@suse.de/
Cc: Laurent Pinchart
Cc: Dave Airlie
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: Sumit Semwal
Cc: "Christian K�nig"
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_gem_dma_helper.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_dma_helper.c
b/drivers/gpu/drm/drm_gem_dma_helper.c
index 1e658c448366..637a5cc62457 100644
--- a/drivers/gpu/drm/drm_gem_dma_helper.c
+++ b/drivers/gpu/drm/drm_gem_dma_helper.c
@@ -525,13 +525,10 @@ int drm_gem_dma_mmap(struct drm_gem_dma_object *dma_obj,
struct vm_area_struct *
int ret;
/*
-* Clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and set the
-* vm_pgoff (used as a fake buffer offset by DRM) to 0 as we want to map
-* the whole buffer.
+* Set the vm_pgoff (used as a fake buffer offset by DRM) to 0 as we
+* want to map the whole buffer.
*/
vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node);
- vma->vm_flags &= ~VM_PFNMAP;
- vma->vm_flags |= VM_DONTEXPAND;
if (dma_obj->map_noncoherent) {
vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
[PATCH 3/3] ASoC: mediatek: mt8186: correct the HDMI widgets
Use SND_SOC_DAPM_LINE instead of SND_SOC_DAPM_OUTPUT to trigger
DAPM events to hdmi-codec when userspace control the DPAM pin.
Signed-off-by: Jiaxin Yu
---
sound/soc/mediatek/mt8186/mt8186-mt6366-da7219-max98357.c | 2 +-
sound/soc/mediatek/mt8186/mt8186-mt6366-rt1019-rt5682s.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/mediatek/mt8186/mt8186-mt6366-da7219-max98357.c
b/sound/soc/mediatek/mt8186/mt8186-mt6366-da7219-max98357.c
index d8c93d66b4e6..f4960b0c8298 100644
--- a/sound/soc/mediatek/mt8186/mt8186-mt6366-da7219-max98357.c
+++ b/sound/soc/mediatek/mt8186/mt8186-mt6366-da7219-max98357.c
@@ -965,7 +965,7 @@ mt8186_mt6366_da7219_max98357_widgets[] = {
SND_SOC_DAPM_SPK("Speakers", NULL),
SND_SOC_DAPM_HP("Headphones", NULL),
SND_SOC_DAPM_MIC("Headset Mic", NULL),
- SND_SOC_DAPM_OUTPUT("HDMI1"),
+ SND_SOC_DAPM_LINE("HDMI1", NULL),
SND_SOC_DAPM_MIXER(SOF_DMA_DL1, SND_SOC_NOPM, 0, 0, NULL, 0),
SND_SOC_DAPM_MIXER(SOF_DMA_DL2, SND_SOC_NOPM, 0, 0, NULL, 0),
SND_SOC_DAPM_MIXER(SOF_DMA_UL1, SND_SOC_NOPM, 0, 0, NULL, 0),
diff --git a/sound/soc/mediatek/mt8186/mt8186-mt6366-rt1019-rt5682s.c
b/sound/soc/mediatek/mt8186/mt8186-mt6366-rt1019-rt5682s.c
index e59d92e2afa3..c61e9bb419d4 100644
--- a/sound/soc/mediatek/mt8186/mt8186-mt6366-rt1019-rt5682s.c
+++ b/sound/soc/mediatek/mt8186/mt8186-mt6366-rt1019-rt5682s.c
@@ -1033,7 +1033,7 @@ mt8186_mt6366_rt1019_rt5682s_widgets[] = {
SND_SOC_DAPM_SPK("Speakers", NULL),
SND_SOC_DAPM_HP("Headphone", NULL),
SND_SOC_DAPM_MIC("Headset Mic", NULL),
- SND_SOC_DAPM_OUTPUT("HDMI1"),
+ SND_SOC_DAPM_LINE("HDMI1", NULL),
SND_SOC_DAPM_MIXER(SOF_DMA_DL1, SND_SOC_NOPM, 0, 0, NULL, 0),
SND_SOC_DAPM_MIXER(SOF_DMA_DL2, SND_SOC_NOPM, 0, 0, NULL, 0),
SND_SOC_DAPM_MIXER(SOF_DMA_UL1, SND_SOC_NOPM, 0, 0, NULL, 0),
--
2.18.0
[PATCH 0/3] ASoC: mediatek:mt8186: fix both the speaker and hdmi
The speaker and hdmi of mt8186 platform are shared the same port of I2S, when connect the external display, use build-in speakers to play audio, they both playback at the same time. So we want to manage the playback device through DAPM events. Jiaxin Yu (3): drm: bridge: it6505: bridge to hdmi-codec ASoC: hdmi-codec: Add event handler for hdmi TX ASoC: mediatek: mt8186: correct the HDMI widgets drivers/gpu/drm/bridge/ite-it6505.c | 36 +++--- include/sound/hdmi-codec.h| 6 +++ sound/soc/codecs/hdmi-codec.c | 37 +-- .../mt8186/mt8186-mt6366-da7219-max98357.c| 2 +- .../mt8186/mt8186-mt6366-rt1019-rt5682s.c | 2 +- 5 files changed, 71 insertions(+), 12 deletions(-) -- 2.18.0
[PATCH 1/3] drm: bridge: it6505: bridge to hdmi-codec
1. Bridge to hdmi-codec to support audio feature. At the same time,
the function of automatically detecting audio is removed.
2. It is observed that some DP-to-HDMI dongles will get into bad
states if sending InfoFrame without audio data. Defer to enable
it6505's audio feature when PCM triggers START or RESUME.
Signed-off-by: Jiaxin Yu
---
drivers/gpu/drm/bridge/ite-it6505.c | 36 -
1 file changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ite-it6505.c
b/drivers/gpu/drm/bridge/ite-it6505.c
index dfe4351c9bdd..f4e1a1d94ed8 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -2159,7 +2159,6 @@ static void it6505_stop_link_train(struct it6505 *it6505)
static void it6505_link_train_ok(struct it6505 *it6505)
{
- struct device *dev = &it6505->client->dev;
it6505->link_state = LINK_OK;
/* disalbe mute enable avi info frame */
@@ -2167,11 +2166,6 @@ static void it6505_link_train_ok(struct it6505 *it6505)
it6505_set_bits(it6505, REG_INFOFRAME_CTRL,
EN_VID_CTRL_PKT, EN_VID_CTRL_PKT);
- if (it6505_audio_input(it6505)) {
- DRM_DEV_DEBUG_DRIVER(dev, "Enable audio!");
- it6505_enable_audio(it6505);
- }
-
if (it6505->hdcp_desired)
it6505_start_hdcp(it6505);
}
@@ -2823,6 +2817,36 @@ static void __maybe_unused it6505_audio_shutdown(struct
device *dev, void *data)
it6505_disable_audio(it6505);
}
+static int __maybe_unused it6505_audio_setup_trigger(struct it6505 *it6505,
int cmd)
+{
+ struct device *dev = &it6505->client->dev;
+
+ DRM_DEV_DEBUG_DRIVER(dev, "event: %d", cmd);
+
+ switch (cmd) {
+ case SNDRV_PCM_TRIGGER_START:
+ case SNDRV_PCM_TRIGGER_RESUME:
+ queue_delayed_work(system_wq, &it6505->delayed_audio,
+ msecs_to_jiffies(180));
+ break;
+ case SNDRV_PCM_TRIGGER_STOP:
+ case SNDRV_PCM_TRIGGER_SUSPEND:
+ cancel_delayed_work(&it6505->delayed_audio);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int __maybe_unused it6505_audio_trigger(struct device *dev, int cmd)
+{
+ struct it6505 *it6505 = dev_get_drvdata(dev);
+
+ return it6505_audio_setup_trigger(it6505, cmd);
+}
+
static int __maybe_unused it6505_audio_hook_plugged_cb(struct device *dev,
void *data,
hdmi_codec_plugged_cb fn,
--
2.18.0
[PATCH 2/3] ASoC: hdmi-codec: Add event handler for hdmi TX
If the speaker and hdmi are connect to the same port of I2S,
when try to switch to speaker playback, we will find that hdmi
is always turned on automatically. The way of switching is
through SOC_DAPM_PIN_SWITCH, however, such events can not be
handled in hdmi-codec driver.
So add event handler for hdmi TX to solve the above issue.
Signed-off-by: Jiaxin Yu
---
include/sound/hdmi-codec.h| 6 ++
sound/soc/codecs/hdmi-codec.c | 37 +++
2 files changed, 39 insertions(+), 4 deletions(-)
diff --git a/include/sound/hdmi-codec.h b/include/sound/hdmi-codec.h
index 48ad33aba393..dcbc09254144 100644
--- a/include/sound/hdmi-codec.h
+++ b/include/sound/hdmi-codec.h
@@ -81,6 +81,12 @@ struct hdmi_codec_ops {
struct hdmi_codec_daifmt *fmt,
struct hdmi_codec_params *hparms);
+ /*
+* PCM trigger callback.
+* Mandatory
+*/
+ int (*trigger)(struct device *dev, int cmd);
+
/*
* Shuts down the audio stream.
* Mandatory
diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index 0b1cdb2d6049..cb4479372e09 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -276,7 +276,31 @@ struct hdmi_codec_priv {
u8 iec_status[AES_IEC958_STATUS_SIZE];
};
+static int hdmi_tx_event(struct snd_soc_dapm_widget *w,
+ struct snd_kcontrol *kcontrol, int event)
+{
+ struct snd_soc_component *component =
snd_soc_dapm_to_component(w->dapm);
+ struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component);
+
+ switch (event) {
+ case SND_SOC_DAPM_PRE_PMU:
+ if (hcp->hcd.ops->trigger)
+ hcp->hcd.ops->trigger(component->dev->parent,
SNDRV_PCM_TRIGGER_START);
+ break;
+ case SND_SOC_DAPM_POST_PMD:
+ if (hcp->hcd.ops->trigger)
+ hcp->hcd.ops->trigger(component->dev->parent,
SNDRV_PCM_TRIGGER_STOP);
+ break;
+ default:
+ break;
+ }
+
+ return 0;
+}
+
static const struct snd_soc_dapm_widget hdmi_widgets[] = {
+ SND_SOC_DAPM_OUT_DRV_E("SDB", SND_SOC_NOPM, 0, 0, NULL, 0,
hdmi_tx_event,
+ SND_SOC_DAPM_POST_PMD | SND_SOC_DAPM_PRE_PMU),
SND_SOC_DAPM_OUTPUT("TX"),
SND_SOC_DAPM_OUTPUT("RX"),
};
@@ -808,18 +832,23 @@ static int hdmi_dai_probe(struct snd_soc_dai *dai)
struct hdmi_codec_daifmt *daifmt;
struct snd_soc_dapm_route route[] = {
{
- .sink = "TX",
+ .sink = dai->driver->capture.stream_name,
+ .source = "RX",
+ },
+ {
+ .sink = "SDB",
.source = dai->driver->playback.stream_name,
},
{
- .sink = dai->driver->capture.stream_name,
- .source = "RX",
+ .sink = "TX",
+ .source = "SDB",
},
+
};
int ret;
dapm = snd_soc_component_get_dapm(dai->component);
- ret = snd_soc_dapm_add_routes(dapm, route, 2);
+ ret = snd_soc_dapm_add_routes(dapm, route, ARRAY_SIZE(route));
if (ret)
return ret;
--
2.18.0
Re: [PATCH v6 6/7] dt/bindings: drm/bridge: it6505: Add mode-switch support
On Thu, 24 Nov 2022 18:20:55 +0800, Pin-yen Lin wrote: > ITE IT6505 can be used in systems to switch the DP traffic between > two downstreams, which can be USB Type-C DisplayPort alternate mode > lane or regular DisplayPort output ports. > > Update the binding to accommodate this usage by introducing a > data-lanes and a mode-switch property on endpoints. > > Signed-off-by: Pin-yen Lin > > --- > > Changes in v6: > - Remove switches node and use endpoints and data-lanes property to > describe the connections. > > .../bindings/display/bridge/ite,it6505.yaml | 94 ++- > 1 file changed, 90 insertions(+), 4 deletions(-) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: ./Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml:99:1: [error] syntax error: found character '\t' that cannot start any token (syntax) dtschema/dtc warnings/errors: make[1]: *** Deleting file 'Documentation/devicetree/bindings/display/bridge/ite,it6505.example.dts' Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml:99:1: found character '\t' that cannot start any token make[1]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/display/bridge/ite,it6505.example.dts] Error 1 make[1]: *** Waiting for unfinished jobs ./Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml:99:1: found character '\t' that cannot start any token /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml: ignoring, error parsing file make: *** [Makefile:1492: dt_binding_check] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221124102056.393220-7-treapk...@chromium.org This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command.
Re: [PATCH v6 3/7] dt-bindings: drm/bridge: anx7625: Add mode-switch support
On Thu, 24 Nov 2022 18:20:52 +0800, Pin-yen Lin wrote: > Analogix 7625 can be used in systems to switch the DP traffic between > two downstreams, which can be USB Type-C DisplayPort alternate mode > lane or regular DisplayPort output ports. > > Update the binding to accommodate this usage by introducing a > data-lanes and a mode-switch property on endpoints. > > Also include the link to the product brief in the bindings. > > Signed-off-by: Pin-yen Lin > > --- > > Changes in v6: > - Remove switches node and use endpoints and data-lanes property to > describe the connections. > > .../display/bridge/analogix,anx7625.yaml | 73 ++- > 1 file changed, 71 insertions(+), 2 deletions(-) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: ./Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml:218:1: [error] syntax error: found character '\t' that cannot start any token (syntax) dtschema/dtc warnings/errors: make[1]: *** Deleting file 'Documentation/devicetree/bindings/display/bridge/analogix,anx7625.example.dts' Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml:218:1: found character '\t' that cannot start any token make[1]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/display/bridge/analogix,anx7625.example.dts] Error 1 make[1]: *** Waiting for unfinished jobs ./Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml:218:1: found character '\t' that cannot start any token /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml: ignoring, error parsing file make: *** [Makefile:1492: dt_binding_check] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221124102056.393220-4-treapk...@chromium.org This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command.
Re: [PATCH v3 04/18] dt-bindings: msm: dsi-controller-main: Add compatible strings for every current SoC
On Thu, 24 Nov 2022 00:47:47 +, Bryan O'Donoghue wrote: > Currently we do not differentiate between the various users of the > qcom,mdss-dsi-ctrl. The driver is flexible enough to operate from one > compatible string but, the hardware does have some significant differences > in the number of clocks. > > To facilitate documenting the clocks add the following compatible strings > > - qcom,apq8064-dsi-ctrl > - qcom,msm8916-dsi-ctrl > - qcom,msm8974-dsi-ctrl > - qcom,msm8996-dsi-ctrl > - qcom,sc7180-dsi-ctrl > - qcom,sc7280-dsi-ctrl > - qcom,sdm630-dsi-ctrl > - qcom,sdm660-dsi-ctrl > - qcom,sdm845-dsi-ctrl > - qcom,sm8250-dsi-ctrl > > Each SoC dtsi should declare "qcom,socname-dsi-ctrl", "qcom,mdss-dsi-ctrl"; > > Signed-off-by: Bryan O'Donoghue > --- > .../display/msm/dsi-controller-main.yaml| 17 ++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/dsi-controller-main.example.dtb: dsi@ae94000: compatible:0: 'qcom,mdss-dsi-ctrl' is not one of ['qcom,apq8064-dsi-ctrl', 'qcom,msm8916-dsi-ctrl', 'qcom,msm8974-dsi-ctrl', 'qcom,msm8996-dsi-ctrl', 'qcom,qcm2290-dsi-ctrl', 'qcom,sc7180-dsi-ctrl', 'qcom,sc7280-dsi-ctrl', 'qcom,sdm630-dsi-ctrl', 'qcom,sdm660-dsi-ctrl', 'qcom,sdm845-dsi-ctrl', 'qcom,sm8250-dsi-ctrl'] From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/dsi-controller-main.example.dtb: dsi@ae94000: compatible: ['qcom,mdss-dsi-ctrl'] is too short From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221124004801.361232-5-bryan.odonog...@linaro.org This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command.
Re: [PATCH] drm/dma-helpers: Don't change vma flags
On Thu, Nov 24, 2022 at 11:11:21AM +, Robin Murphy wrote:
> On 2022-11-23 17:28, Daniel Vetter wrote:
> > This code was added in b65e64f7ccd4 ("drm/cma: Use
> > dma_mmap_writecombine() to mmap buffer"), but does not explain why
> > it's needed.
> >
> > It should be entirely unnecessary, because remap_pfn_range(), which is
> > what the various dma_mmap functiosn are built on top of, does already
> > unconditionally adjust the vma flags:
>
> Not all dma_mmap_*() implementations use remap_pfn_range() though. For
> instance on ARM where one set of DMA ops uses vm_map_pages(), but AFAICS not
> all the relevant drivers would set VM_MIXEDMAP to prevent reaching the
> BUG_ON(vma->vm_flags & VM_PFNMAP) in there.
Uh a dma_mmap which does not use VM_PFNMAP has pretty good chances of
being busted, since that allows get_user_pages on these memory
allocations. And I'm really not sure that's a bright idea ...
Can you please point me at these dma-ops so that I can try and understand
what they're trying to do?
-Daniel
>
> Robin.
>
> > https://elixir.bootlin.com/linux/v6.1-rc6/source/mm/memory.c#L2518
> >
> > More importantly, it does uncondtionally set VM_PFNMAP, so clearing
> > that does not make much sense.
> >
> > Patch motived by discussions around enforcing VM_PFNMAP semantics for
> > all dma-buf users, where Thomas asked why dma helpers will work with
> > that dma_buf_mmap() contract.
> >
> > References:
> > https://lore.kernel.org/dri-devel/5c3c8d4f-2c06-9210-b00a-4d0ff6f6f...@suse.de/
> > Cc: Laurent Pinchart
> > Cc: Dave Airlie
> > Cc: Maarten Lankhorst
> > Cc: Maxime Ripard
> > Cc: Thomas Zimmermann
> > Cc: Sumit Semwal
> > Cc: "Christian König"
> > Signed-off-by: Daniel Vetter
> > ---
> > drivers/gpu/drm/drm_gem_dma_helper.c | 7 ++-
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_gem_dma_helper.c
> > b/drivers/gpu/drm/drm_gem_dma_helper.c
> > index 1e658c448366..637a5cc62457 100644
> > --- a/drivers/gpu/drm/drm_gem_dma_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_dma_helper.c
> > @@ -525,13 +525,10 @@ int drm_gem_dma_mmap(struct drm_gem_dma_object
> > *dma_obj, struct vm_area_struct *
> > int ret;
> > /*
> > -* Clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and set the
> > -* vm_pgoff (used as a fake buffer offset by DRM) to 0 as we want to map
> > -* the whole buffer.
> > +* Set the vm_pgoff (used as a fake buffer offset by DRM) to 0 as we
> > +* want to map the whole buffer.
> > */
> > vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node);
> > - vma->vm_flags &= ~VM_PFNMAP;
> > - vma->vm_flags |= VM_DONTEXPAND;
> > if (dma_obj->map_noncoherent) {
> > vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH AUTOSEL 6.0 38/44] drm/amdgpu: Unlock bo_list_mutex after error handling
On Mon, Nov 21, 2022 at 12:07:40PM +0100, Christian König wrote:
Am 21.11.22 um 10:57 schrieb Michel Dänzer:
On 11/19/22 03:11, Sasha Levin wrote:
From: Philip Yang
[ Upstream commit 64f65135c41a75f933d3bca236417ad8e9eb75de ]
Get below kernel WARNING backtrace when pressing ctrl-C to kill kfdtest
application.
If amdgpu_cs_parser_bos returns error after taking bo_list_mutex, as
caller amdgpu_cs_ioctl will not unlock bo_list_mutex, this generates the
kernel WARNING.
Add unlock bo_list_mutex after amdgpu_cs_parser_bos error handling to
cleanup bo_list userptr bo.
WARNING: kfdtest/2930 still has locks held!
1 lock held by kfdtest/2930:
(&list->bo_list_mutex){+.+.}-{3:3}, at: amdgpu_cs_ioctl+0xce5/0x1f10 [amdgpu]
stack backtrace:
dump_stack_lvl+0x44/0x57
get_signal+0x79f/0xd00
arch_do_signal_or_restart+0x36/0x7b0
exit_to_user_mode_prepare+0xfd/0x1b0
syscall_exit_to_user_mode+0x19/0x40
do_syscall_64+0x40/0x80
Signed-off-by: Philip Yang
Reviewed-by: Christian König
Signed-off-by: Alex Deucher
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index b7bae833c804..9d59f83c8faa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -655,6 +655,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
}
mutex_unlock(&p->bo_list->bo_list_mutex);
}
+ mutex_unlock(&p->bo_list->bo_list_mutex);
return r;
}
Looks doubtful that this is a correct backport — there's an identical
mutex_unlock call just above.
Oh, yes good point. This patch doesn't needs to be backported at all
because it just fixes a problem introduced in the same cycle:
Dropping it, thanks!
--
Thanks,
Sasha
Re: [PATCH v2] drm/amdgpu: Fix potential double free and null pointer dereference
Applied.
Regards,
Luben
On 2022-11-22 19:10, Luben Tuikov wrote:
> amdgpu_xgmi_hive_type does provide a release method which frees the allocated
> "hive",
> so we don't need a kfree() after a kobject_put().
>
> Reviewed-by: Luben Tuikov
>
> Regards,
> Luben
>
> On 2022-11-21 23:28, Liang He wrote:
>> In amdgpu_get_xgmi_hive(), we should not call kfree() after
>> kobject_put() as the PUT will call kfree().
>>
>> In amdgpu_device_ip_init(), we need to check the returned *hive*
>> which can be NULL before we dereference it.
>>
>> Signed-off-by: Liang He
>> ---
>> v1->v2: we need the extra GET to keep *hive* alive, it is
>> my fault to remove the GET in v1.
>>
>>
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 5 +
>> drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 2 --
>> 2 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index f1e9663b4051..00976e15b698 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -2462,6 +2462,11 @@ static int amdgpu_device_ip_init(struct amdgpu_device
>> *adev)
>> if (!amdgpu_sriov_vf(adev)) {
>> struct amdgpu_hive_info *hive =
>> amdgpu_get_xgmi_hive(adev);
>>
>> +if (WARN_ON(!hive)) {
>> +r = -ENOENT;
>> +goto init_failed;
>> +}
>> +
>> if (!hive->reset_domain ||
>>
>> !amdgpu_reset_get_reset_domain(hive->reset_domain)) {
>> r = -ENOENT;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
>> index 47159e9a0884..4b9e7b050ccd 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
>> @@ -386,7 +386,6 @@ struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct
>> amdgpu_device *adev)
>> if (ret) {
>> dev_err(adev->dev, "XGMI: failed initializing kobject for xgmi
>> hive\n");
>> kobject_put(&hive->kobj);
>> -kfree(hive);
>> hive = NULL;
>> goto pro_end;
>> }
>> @@ -410,7 +409,6 @@ struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct
>> amdgpu_device *adev)
>> dev_err(adev->dev, "XGMI: failed initializing
>> reset domain for xgmi hive\n");
>> ret = -ENOMEM;
>> kobject_put(&hive->kobj);
>> -kfree(hive);
>> hive = NULL;
>> goto pro_end;
>> }
>
Re: [RESEND 2/6 v2] media: uapi: Document format MEDIA_BUS_FMT_RGB565_1X24_CPADHI
On 19/05/2022 18:29, Chris Morgan wrote: > From: Chris Morgan > > Add support for MEDIA_BUS_FMT_RGB565_1X24_CPADHI. This format is used > by the Geekworm MZP280 panel which interfaces with the Raspberry Pi. > > Signed-off-by: Chris Morgan > --- > .../media/v4l/subdev-formats.rst | 37 +++ > 1 file changed, 37 insertions(+) > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst > b/Documentation/userspace-api/media/v4l/subdev-formats.rst > index 0cbc045d5..e43e07634 100644 > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst > @@ -624,6 +624,43 @@ The following tables list existing packed RGB formats. >- b\ :sub:`2` >- b\ :sub:`1` >- b\ :sub:`0` > +* .. _MEDIA_BUS_FMT_RGB565_1X24_CPADHI: > + > + - MEDIA_BUS_FMT_RGB565_1X24_CPADHI > + - 0x101e Given the latest media tree this is now 0x1022. Otherwise: Acked-by: Hans Verkuil It's an old series by now, but as far as I am concerned this can go through the drm subsystem (after updating this value). Regards, Hans > + - > + - > + - > + - > + - > + - > + - > + - > + - > + - 0 > + - 0 > + - 0 > + - r\ :sub:`4` > + - r\ :sub:`3` > + - r\ :sub:`2` > + - r\ :sub:`1` > + - r\ :sub:`0` > + - 0 > + - 0 > + - g\ :sub:`5` > + - g\ :sub:`4` > + - g\ :sub:`3` > + - g\ :sub:`2` > + - g\ :sub:`1` > + - g\ :sub:`0` > + - 0 > + - 0 > + - 0 > + - b\ :sub:`4` > + - b\ :sub:`3` > + - b\ :sub:`2` > + - b\ :sub:`1` > + - b\ :sub:`0` > * .. _MEDIA-BUS-FMT-BGR565-2X8-BE: > >- MEDIA_BUS_FMT_BGR565_2X8_BE
Re: [RESEND 3/6 v2] media: uapi: add MEDIA_BUS_FMT_RGB565_1X24_CPADHI
On 19/05/2022 18:29, Chris Morgan wrote: > From: Chris Morgan > > Add the MEDIA_BUS_FMT_RGB565_1X24_CPADHI format used by the Geekworm > MZP280 panel for the Raspberry Pi. > > Signed-off-by: Chris Morgan > --- > include/uapi/linux/media-bus-format.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/uapi/linux/media-bus-format.h > b/include/uapi/linux/media-bus-format.h > index 0dfc11ee2..a7b765498 100644 > --- a/include/uapi/linux/media-bus-format.h > +++ b/include/uapi/linux/media-bus-format.h > @@ -34,13 +34,14 @@ > > #define MEDIA_BUS_FMT_FIXED 0x0001 > > -/* RGB - next is 0x101e */ > +/* RGB - next is 0x101f */ This would be 0x1023 > #define MEDIA_BUS_FMT_RGB444_1X120x1016 > #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE0x1001 > #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE0x1002 > #define MEDIA_BUS_FMT_RGB555_2X8_PADHI_BE0x1003 > #define MEDIA_BUS_FMT_RGB555_2X8_PADHI_LE0x1004 > #define MEDIA_BUS_FMT_RGB565_1X160x1017 > +#define MEDIA_BUS_FMT_RGB565_1X24_CPADHI 0x101e and this 0x1022, based on the latest media tree. > #define MEDIA_BUS_FMT_BGR565_2X8_BE 0x1005 > #define MEDIA_BUS_FMT_BGR565_2X8_LE 0x1006 > #define MEDIA_BUS_FMT_RGB565_2X8_BE 0x1007 Otherwise: Acked-by: Hans Verkuil It's an old series by now, but as far as I am concerned this can go through the drm subsystem (after updating this value). Regards, Hans
Re: [PATCH 00/23] Add Tegra20 parallel video input capture
HI Hans, On Thu, 24 Nov 2022 09:36:14 +0100 Hans Verkuil wrote: > Hi Luca, > > On 09/11/2022 15:18, luca.ceres...@bootlin.com wrote: > > From: Luca Ceresoli > > > > Tegra20 and other Tegra SoCs have a video input (VI) peripheral that can > > receive from either MIPI CSI-2 or parallel video (called respectively "CSI" > > and "VIP" in the documentation). The kernel currently has a staging driver > > for Tegra210 CSI capture. This patch set adds support for Tegra20 VIP > > capture. > > > > Unfortunately I had no real documentation available to base this work on, > > and I started from a working downstream 3.1 kernel, that I have heavily > > reworked to fit into the existing tegra-video driver. The existing code > > appears written with the intent of being modular and allow adding new input > > mechanisms and new SoCs while keeping a unique VI core module. However its > > modularity and extensibility was not enough to add Tegra20 VIP support, so > > I added some hooks to turn hard-coded behaviour into per-SoC or per-bus > > customizable code. There are also some fixes, some generic cleanups and DT > > bindings. > > I plan on testing this series (esp. making sure it keeps working on > non-tegra210 > hardware), but I have to find time for that. This is on my TODO list, so I > will > get to it, but it might time. Thanks for letting me know! Indeed, with respect to what you wrote, even testing on tegra210 hardware would be useful in case you have any, as all the hardware I can access is tegra20. Kind regards. -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
igt-kms-rockchip regressions since v6.0-rc1
Hi Sandy, Going though the KernelCI results I noticed that the v6.0-rc1 kernel introduced a number of regressions in the igt-kms-rockchip tests: https://groups.io/g/kernelci-results/message/34239?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3ACreated%2C%2Cigt-kms-rockchip%2C20%2C1%2C80%2C95118839 I don't know if anyone is taking a look at this, so I figured out you could be interested. Thanks, Ricardo
Re: [PATCH 00/24] drm: Introduce Kunit Tests to VC4
Hi David, On Thu, Nov 24, 2022 at 04:31:14PM +0800, David Gow wrote: > On Wed, Nov 23, 2022 at 11:28 PM Maxime Ripard wrote: > > > > Hi, > > > > This series introduce Kunit tests to the vc4 KMS driver, but unlike what we > > have been doing so far in KMS, it actually tests the atomic modesetting > > code. > > > > In order to do so, I've had to improve a fair bit on the Kunit helpers > > already > > found in the tree in order to register a full blown and somewhat functional > > KMS > > driver. > > > > It's of course relying on a mock so that we can test it anywhere. The > > mocking > > approach created a number of issues, the main one being that we need to > > create > > a decent mock in the first place, see patch 22. The basic idea is that I > > created some structures to provide a decent approximation of the actual > > hardware, and that would support both major architectures supported by vc4. > > > > This is of course meant to evolve over time and support more tests, but I've > > focused on testing the HVS FIFO assignment code which is fairly tricky (and > > the > > tests have actually revealed one more bug with our current implementation). > > I > > used to have a userspace implementation of those tests, where I would copy > > and > > paste the kernel code and run the tests on a regular basis. It's was > > obviously > > fairly suboptimal, so it seemed like the perfect testbed for that series. > > Thanks very much for this! I'm really excited to see these sorts of > tests being written. > > I was able to successfully run these under qemu with: > ./tools/testing/kunit/kunit.py run --kunitconfig > drivers/gpu/drm/vc4/tests --arch arm64 > --cross_compile=aarch64-linux-gnu- > (and also with clang, using --make_options LLVM=1 instead of the > --cross_compile flag) > > On the other hand, they don't compile as a module: > ERROR: modpost: missing MODULE_LICENSE() in > drivers/gpu/drm/vc4/tests/vc4_mock.o > ERROR: modpost: missing MODULE_LICENSE() in > drivers/gpu/drm/vc4/tests/vc4_mock_crtc.o > ERROR: modpost: missing MODULE_LICENSE() in > drivers/gpu/drm/vc4/tests/vc4_mock_output.o > ERROR: modpost: missing MODULE_LICENSE() in > drivers/gpu/drm/vc4/tests/vc4_mock_plane.o > ERROR: modpost: missing MODULE_LICENSE() in > drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.o > ERROR: modpost: missing MODULE_LICENSE() in > drivers/gpu/drm/tests/drm_managed_test.o > ERROR: modpost: "vc4_drm_driver" > [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! > ERROR: modpost: "vc5_drm_driver" > [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! > ERROR: modpost: "drm_kunit_helper_alloc_device" > [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! > ERROR: modpost: "__drm_kunit_helper_alloc_drm_device_with_driver" > [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! > ERROR: modpost: "__vc4_hvs_alloc" > [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! > ERROR: modpost: "vc4_dummy_plane" > [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! > ERROR: modpost: "vc4_mock_pv" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] > undefined! > ERROR: modpost: "vc4_dummy_output" > [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! > ERROR: modpost: "vc4_kms_load" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] > undefined! > ERROR: modpost: "vc4_txp_crtc_data" > [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! > WARNING: modpost: suppressed 17 unresolved symbol warnings because > there were too many) Thanks I'll fix it > Most of those are just the need to export some symbols. There's some > work underway to support conditionally exporting symbols only if KUnit > is enabled, which may help: > https://lore.kernel.org/linux-kselftest/20221102175959.2921063-1-rm...@google.com/ That's awesome :) The current solution to include the test implementation is not ideal, so it's great to see a nicer solution being worked on. > Otherwise, I suspect the better short-term solution would just be to > require that the tests are built-in (or at least compiled into > whatever of the drm/vc4 modules makes most sense). > > The only other thing which has me a little confused is the naming of > some of the functions, specifically with the __ prefix. Is it just for > internal functions (many of them aren't static, but maybe they could > use the VISIBLE_IF_KUNIT macro if that makes sense), or for versions > of functions which accept extra arguments? It was for internal functions that would definitely benefit from VISIBLE_IF_KUNIT indeed > Not a big deal (and maybe it's a DRM naming convention I'm ignorant > of), but I couldn't quite find a pattern on my first read through. > > But on the whole, these look good from a KUnit point-of-view. It's > really to see some solid mocking and driver testing, too. There would > be ways to avoid passing the 'struct kunit' around in more places (or > to store extra data as a kunit_resource), but I think it's better > overall to pass it around like you have in this case -- it's certainly > more compatible with things which m
Re: [PATCH v10 06/19] drm/modes: Add a function to generate analog display modes
Den 17.11.2022 10.28, skrev Maxime Ripard: > Multiple drivers (meson, vc4, sun4i) define analog TV 525-lines and > 625-lines modes in their drivers. > > Since those modes are fairly standard, and that we'll need to use them > in more places in the future, it makes sense to move their definition > into the core framework. > > However, analog display usually have fairly loose timings requirements, > the only discrete parameters being the total number of lines and pixel > clock frequency. Thus, we created a function that will create a display > mode from the standard, the pixel frequency and the active area. > > Tested-by: Mateusz Kwiatkowski > Signed-off-by: Maxime Ripard > > --- I'm no domain expert so apart from the timing details which I can't comment on, it looks fine. I personally advocated for a much simpler solution for these NTSC and PAL modes, but AIUI this is part of a grander plan to support devices with other timings. Acked-by: Noralf Trønnes
Re: [PATCH v10 05/19] drm/connector: Add TV standard property
Den 17.11.2022 10.28, skrev Maxime Ripard:
> The TV mode property has been around for a while now to select and get the
> current TV mode output on an analog TV connector.
>
> Despite that property name being generic, its content isn't and has been
> driver-specific which makes it hard to build any generic behaviour on top
> of it, both in kernel and user-space.
>
> Let's create a new enum tv norm property, that can contain any of the
> analog TV standards currently supported by kernel drivers. Each driver can
> then pass in a bitmask of the modes it supports, and the property
> creation function will filter out the modes not supported.
>
> We'll then be able to phase out the older tv mode property.
>
> Tested-by: Mateusz Kwiatkowski
> Signed-off-by: Maxime Ripard
>
> ---
> Changes in v10:
> - Fix checkpatch warning
>
> Changes in v5:
> - Create an analog TV properties documentation section, and document TV
> Mode there instead of the csv file
>
> Changes in v4:
> - Add property documentation to kms-properties.csv
> - Fix documentation
> ---
> Documentation/gpu/drm-kms.rst | 6 ++
> drivers/gpu/drm/drm_atomic_uapi.c | 4 ++
> drivers/gpu/drm/drm_connector.c | 122
> +-
> include/drm/drm_connector.h | 64
> include/drm/drm_mode_config.h | 8 +++
> 5 files changed, 203 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index b4377a545425..321f2f582c64 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -520,6 +520,12 @@ HDMI Specific Connector Properties
> .. kernel-doc:: drivers/gpu/drm/drm_connector.c
> :doc: HDMI connector properties
>
> +Analog TV Specific Connector Properties
> +--
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_connector.c
> + :doc: Analog TV Connector Properties
> +
> Standard CRTC Properties
>
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 7f2b9a07fbdf..d867e7f9f2cd 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -700,6 +700,8 @@ static int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> state->tv.margins.bottom = val;
> } else if (property == config->legacy_tv_mode_property) {
> state->tv.legacy_mode = val;
> + } else if (property == config->tv_mode_property) {
> + state->tv.mode = val;
> } else if (property == config->tv_brightness_property) {
> state->tv.brightness = val;
> } else if (property == config->tv_contrast_property) {
> @@ -810,6 +812,8 @@ drm_atomic_connector_get_property(struct drm_connector
> *connector,
> *val = state->tv.margins.bottom;
> } else if (property == config->legacy_tv_mode_property) {
> *val = state->tv.legacy_mode;
> + } else if (property == config->tv_mode_property) {
> + *val = state->tv.mode;
> } else if (property == config->tv_brightness_property) {
> *val = state->tv.brightness;
> } else if (property == config->tv_contrast_property) {
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 06e737ed15f5..07d449736956 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -984,6 +984,17 @@ static const struct drm_prop_enum_list
> drm_dvi_i_subconnector_enum_list[] = {
> DRM_ENUM_NAME_FN(drm_get_dvi_i_subconnector_name,
>drm_dvi_i_subconnector_enum_list)
>
> +static const struct drm_prop_enum_list drm_tv_mode_enum_list[] = {
> + { DRM_MODE_TV_MODE_NTSC, "NTSC" },
> + { DRM_MODE_TV_MODE_NTSC_443, "NTSC-443" },
> + { DRM_MODE_TV_MODE_NTSC_J, "NTSC-J" },
> + { DRM_MODE_TV_MODE_PAL, "PAL" },
> + { DRM_MODE_TV_MODE_PAL_M, "PAL-M" },
> + { DRM_MODE_TV_MODE_PAL_N, "PAL-N" },
> + { DRM_MODE_TV_MODE_SECAM, "SECAM" },
> +};
> +DRM_ENUM_NAME_FN(drm_get_tv_mode_name, drm_tv_mode_enum_list)
> +
This patch looks good but since I'm no TV standards expert I can't say
if the content of this list is a good choice for reflecting the world of
TV standards.
Acked-by: Noralf Trønnes
> static const struct drm_prop_enum_list drm_tv_select_enum_list[] = {
> { DRM_MODE_SUBCONNECTOR_Automatic, "Automatic" }, /* DVI-I and TV-out */
> { DRM_MODE_SUBCONNECTOR_Composite, "Composite" }, /* TV-out */
> @@ -1552,6 +1563,71 @@
> EXPORT_SYMBOL(drm_connector_attach_dp_subconnector_property);
> * infoframe values is done through drm_hdmi_avi_infoframe_content_type().
> */
>
> +/*
> + * TODO: Document the properties:
> + * - left margin
> + * - right margin
> + * - top margin
> + * - bottom margin
> + * - brightness
> + * - contrast
> + * - flicker reduction
> + * - hue
> + * - mode
> + * - overscan
> + *
Re: [Linaro-mm-sig] Re: [PATCH] dma-buf: Fix possible UAF in dma_buf_export
Am 24.11.22 um 13:49 schrieb cuigaosheng:
I was already wondering why the order is this way.
Why is dma_buf_stats_setup() needing the file in the first place?
dmabuf->file will be used in dma_buf_stats_setup(), the
dma_buf_stats_setup() as follows:
171 int dma_buf_stats_setup(struct dma_buf *dmabuf)
172 {
173 struct dma_buf_sysfs_entry *sysfs_entry;
174 int ret;
175
176 if (!dmabuf || !dmabuf->file)
177 return -EINVAL;
178
179 if (!dmabuf->exp_name) {
180 pr_err("exporter name must not be empty if stats
needed\n");
181 return -EINVAL;
182 }
183
184 sysfs_entry = kzalloc(sizeof(struct dma_buf_sysfs_entry),
GFP_KERNEL);
185 if (!sysfs_entry)
186 return -ENOMEM;
187
188 sysfs_entry->kobj.kset = dma_buf_per_buffer_stats_kset;
189 sysfs_entry->dmabuf = dmabuf;
190
191 dmabuf->sysfs_entry = sysfs_entry;
192
193 /* create the directory for buffer stats */
194 ret = kobject_init_and_add(&sysfs_entry->kobj,
&dma_buf_ktype, NULL,
195 "%lu",
file_inode(dmabuf->file)->i_ino);
Ah, so it uses the i_ino of the file for the sysfs unique name.
I'm going to take another look how to properly clean this up.
Thanks for pointing this out,
Christian.
196 if (ret)
197 goto err_sysfs_dmabuf;
198
199 return 0;
200
201 err_sysfs_dmabuf:
202 kobject_put(&sysfs_entry->kobj);
203 dmabuf->sysfs_entry = NULL;
204 return ret;
205 }
Did I miss something?
Thanks.
On 2022/11/24 20:37, Christian König wrote:
Am 24.11.22 um 13:05 schrieb cuigaosheng:
Some tips:
Before we call the dma_buf_stats_setup(), we have to finish
creating the file,
otherwise dma_buf_stats_setup() will return -EINVAL, maybe we need
to think about
this when making a new patch.
I was already wondering why the order is this way.
Why is dma_buf_stats_setup() needing the file in the first place?
Thanks,
Christian.
Hope these tips are useful, thanks!
On 2022/11/24 13:56, Charan Teja Kalla wrote:
Thanks T.J and Christian for the inputs.
On 11/19/2022 7:00 PM, Christian König wrote:
Yes, exactly that's the idea.
The only alternatives I can see would be to either move
allocating
the
file and so completing the dma_buf initialization last again
or just
ignore errors from sysfs.
> If we still want to avoid calling
dmabuf->ops->release(dmabuf) in
> dma_buf_release like the comment says I guess we could use
sysfs_entry
> and ERR_PTR to flag that, otherwise it looks like we'd
need a bit
> somewhere.
No, this should be dropped as far as I can see. The sysfs
cleanup
code
looks like it can handle not initialized kobj pointers.
Yeah there is also the null check in dma_buf_stats_teardown() that
would prevent it from running, but I understood the comment to be
referring to the release() dma_buf_ops call into the exporter which
comes right after the teardown call. That looks like it's preventing
the fput task work calling back into the exporter after the exporter
already got an error from dma_buf_export(). Otherwise the exporter
sees a release() for a buffer that it doesn't know about / thinks
shouldn't exist. So I could imagine an exporter trying to double
free:
once for the failed dma_buf_export() call, and again when the
release() op is called later.
Oh, very good point as well. Yeah, then creating the file should
probably come last.
@Gaosheng: Could you please make these changes or you let me to do?
Regards,
Christian.
.
.
___
Linaro-mm-sig mailing list -- linaro-mm-...@lists.linaro.org
To unsubscribe send an email to linaro-mm-sig-le...@lists.linaro.org
Re: [PATCH] dma-buf: Fix possible UAF in dma_buf_export
I was already wondering why the order is this way.
Why is dma_buf_stats_setup() needing the file in the first place?
dmabuf->file will be used in dma_buf_stats_setup(), the
dma_buf_stats_setup() as follows:
171 int dma_buf_stats_setup(struct dma_buf *dmabuf)
172 {
173 struct dma_buf_sysfs_entry *sysfs_entry;
174 int ret;
175
176 if (!dmabuf || !dmabuf->file)
177 return -EINVAL;
178
179 if (!dmabuf->exp_name) {
180 pr_err("exporter name must not be empty if stats
needed\n");
181 return -EINVAL;
182 }
183
184 sysfs_entry = kzalloc(sizeof(struct dma_buf_sysfs_entry),
GFP_KERNEL);
185 if (!sysfs_entry)
186 return -ENOMEM;
187
188 sysfs_entry->kobj.kset = dma_buf_per_buffer_stats_kset;
189 sysfs_entry->dmabuf = dmabuf;
190
191 dmabuf->sysfs_entry = sysfs_entry;
192
193 /* create the directory for buffer stats */
194 ret = kobject_init_and_add(&sysfs_entry->kobj,
&dma_buf_ktype, NULL,
195 "%lu",
file_inode(dmabuf->file)->i_ino);
196 if (ret)
197 goto err_sysfs_dmabuf;
198
199 return 0;
200
201 err_sysfs_dmabuf:
202 kobject_put(&sysfs_entry->kobj);
203 dmabuf->sysfs_entry = NULL;
204 return ret;
205 }
Did I miss something?
Thanks.
On 2022/11/24 20:37, Christian König wrote:
Am 24.11.22 um 13:05 schrieb cuigaosheng:
Some tips:
Before we call the dma_buf_stats_setup(), we have to finish
creating the file,
otherwise dma_buf_stats_setup() will return -EINVAL, maybe we need to
think about
this when making a new patch.
I was already wondering why the order is this way.
Why is dma_buf_stats_setup() needing the file in the first place?
Thanks,
Christian.
Hope these tips are useful, thanks!
On 2022/11/24 13:56, Charan Teja Kalla wrote:
Thanks T.J and Christian for the inputs.
On 11/19/2022 7:00 PM, Christian König wrote:
Yes, exactly that's the idea.
The only alternatives I can see would be to either move
allocating
the
file and so completing the dma_buf initialization last again
or just
ignore errors from sysfs.
> If we still want to avoid calling
dmabuf->ops->release(dmabuf) in
> dma_buf_release like the comment says I guess we could use
sysfs_entry
> and ERR_PTR to flag that, otherwise it looks like we'd need
a bit
> somewhere.
No, this should be dropped as far as I can see. The sysfs
cleanup
code
looks like it can handle not initialized kobj pointers.
Yeah there is also the null check in dma_buf_stats_teardown() that
would prevent it from running, but I understood the comment to be
referring to the release() dma_buf_ops call into the exporter which
comes right after the teardown call. That looks like it's preventing
the fput task work calling back into the exporter after the exporter
already got an error from dma_buf_export(). Otherwise the exporter
sees a release() for a buffer that it doesn't know about / thinks
shouldn't exist. So I could imagine an exporter trying to double
free:
once for the failed dma_buf_export() call, and again when the
release() op is called later.
Oh, very good point as well. Yeah, then creating the file should
probably come last.
@Gaosheng: Could you please make these changes or you let me to do?
Regards,
Christian.
.
.
Re: [PATCH] dma-buf: Fix possible UAF in dma_buf_export
Am 24.11.22 um 13:05 schrieb cuigaosheng: Some tips: Before we call the dma_buf_stats_setup(), we have to finish creating the file, otherwise dma_buf_stats_setup() will return -EINVAL, maybe we need to think about this when making a new patch. I was already wondering why the order is this way. Why is dma_buf_stats_setup() needing the file in the first place? Thanks, Christian. Hope these tips are useful, thanks! On 2022/11/24 13:56, Charan Teja Kalla wrote: Thanks T.J and Christian for the inputs. On 11/19/2022 7:00 PM, Christian König wrote: Yes, exactly that's the idea. The only alternatives I can see would be to either move allocating the file and so completing the dma_buf initialization last again or just ignore errors from sysfs. > If we still want to avoid calling dmabuf->ops->release(dmabuf) in > dma_buf_release like the comment says I guess we could use sysfs_entry > and ERR_PTR to flag that, otherwise it looks like we'd need a bit > somewhere. No, this should be dropped as far as I can see. The sysfs cleanup code looks like it can handle not initialized kobj pointers. Yeah there is also the null check in dma_buf_stats_teardown() that would prevent it from running, but I understood the comment to be referring to the release() dma_buf_ops call into the exporter which comes right after the teardown call. That looks like it's preventing the fput task work calling back into the exporter after the exporter already got an error from dma_buf_export(). Otherwise the exporter sees a release() for a buffer that it doesn't know about / thinks shouldn't exist. So I could imagine an exporter trying to double free: once for the failed dma_buf_export() call, and again when the release() op is called later. Oh, very good point as well. Yeah, then creating the file should probably come last. @Gaosheng: Could you please make these changes or you let me to do? Regards, Christian. .
Re: [PATCH v3 2/2] drm/i915: Never return 0 if not all requests retired
On 23/11/2022 16:21, Janusz Krzysztofik wrote:
On Wednesday, 23 November 2022 13:57:26 CET Tvrtko Ursulin wrote:
On 23/11/2022 09:28, Janusz Krzysztofik wrote:
Hi Tvrtko,
Thanks for your comments.
On Tuesday, 22 November 2022 11:50:38 CET Tvrtko Ursulin wrote:
On 21/11/2022 14:56, Janusz Krzysztofik wrote:
Users of intel_gt_retire_requests_timeout() expect 0 return value on
success. However, we have no protection from passing back 0 potentially
returned by a call to dma_fence_wait_timeout() when it succedes right
after its timeout has expired.
Is this talking about a potential weakness, or ambiguous kerneldoc, of
dma_fence_wait_timeout, dma_fence_default_wait and
i915_request_wait_timeout? They appear to say 0 return means timeout,
implying unsignaled fence. In other words signaled must return positive
remaining timeout. Implementations seems to allow a race which indeed
appears that return 0 and signaled fence is possible.
While my initial analysis was indeed focused on inconsistent semantics of 0
return values from different dma_fence_default_wait() backends, I should have
also mentioned in this commit description that users may perfectly
call intel_gt_retire_requests_timeout() with 0 timeout, in which case the
false positive 0 value can be returned regardless of dma_fence_wait_timeout()
potential issues. Would you like me to reword and resubmit?
Not sure yet.
So the only caller which passes in zero to
intel_gt_retire_requests_timeout appears to be intel_gt_retire_requests
and it eats the return value anyway so this patch is immaterial for that
one.
Right.
I guess it can change how intel_gt_wait_for_idle behaves with short-ish
timeouts. In case this race is hit. But then wouldn't it make sense to
follow up with a patch which addresses this race by re-checking the "is
signaled" when timeout expires,
But inside intel_gt_retire_requests_timeout() we generally don't care if
fences have been signaled. As long as user requested timeout hasn't expired,
we use dma_fence_wait_timeout() as an aid, otherwise we keep trying to retire
requests without waiting on fences. If the retirement succeeds then we return
0 (success) regardless of what the return value from the last called
dma_fence_wait_timeout() was. If it was 0 then the only useful information is
that no more time has been left, and no matter if that 0 meant signaled or not
signaled, we must return an error if there are still some requests not
retired, I believe.
Yes I agree with all that. Sorry if my reply was misleading, I mentioned
a follow-up, didn't mean that these two are not ready to go.
either in dma_fence_wait_timeout, or to
intel_gt_retire_requests_timeout. Or if not that at least better
document the dma_fence_wait_timeout and/or
intel_gt_retire_requests_timeout. Makes sense?
Documenting -- yes, as soon as we get into an agreement on what's the core of
this issue -- whether that potential weakness, or ambiguous kerneldoc, of
dma_fence_wait_timeout, dma_fence_default_wait and i915_request_wait_timeout,
as you've stated, that we have to address somehow, or potentially incorrect
direct use of the timeout variable, intended for storing time left to spend on
fence waits, as our return value when timeout has expired. And if the former
then maybe we should also try to finally resolve that over a year old conflict
(whether 0 means signaled on not signaled) inside our implementation of
dma_fence_ops.wait, and simply use a wrapper around it for either our internal
use if we decide to follow the reference implementation, or for dma_fence_ops
use otherwise. Or maybe the reference implementation should be fixed if
problematic. I don't feel competent enough to decide.
Right, we can leave that for later. I'll pull these two in if someone
hasn't already.
Regards,
Tvrtko
Thanks,
Janusz
Regards,
Tvrtko
If dma_fence_wait can indeed return 0 even when a request is signaled,
then how is timeout ?: -ETIME below correct? It isn't a chance for false
negative in its' callers?
The goal of intel_gt_retire_requests_timeout() is to retire requests. When
that goal has been reached, i.e., all requests have been retired, active count
is 0, and 0 is correctly returned, regardless of timeout value.
The value of timeout is used only when there are still pending requests, which
means that the goal hasn't been reached and the function hasn't succeeded.
Then, no false negative is possible, unlike the false positive that we now
have when we return 0 while some requests are still pending.
Thanks,
Janusz
Regards,
Tvrtko
Replace 0 with -ETIME before potentially using the timeout value as return
code, so -ETIME is returned if there are still some requests not retired
after timeout, 0 otherwise.
v3: Use conditional expression, more compact but also better reflecting
intention standing behind the change.
v2: Move the added lines down so flush_submission() is not affected.
Fixes: f33a8a51602c ("drm/i915: Merge wai
Re: [PATCH v6 2/7] platform/chrome: cros_ec_typec: Purge blocking switch devlinks
On Thu, Nov 24, 2022 at 06:20:51PM +0800, Pin-yen Lin wrote: > From: Prashant Malani > > When using OF graph, the fw_devlink code will create links between the > individual port driver (cros-ec-typec here) and the parent device for > a Type-C switch (like mode-switch). Since the mode-switch will in turn > have the usb-c-connector (i.e the child of the port driver) as a > supplier, fw_devlink will not be able to resolve the cyclic dependency > correctly. > > As a result, the mode-switch driver probe() never runs, so mode-switches > are never registered. Because of that, the port driver probe constantly > fails with -EPROBE_DEFER, because the Type-C connector class requires all > switch devices to be registered prior to port registration. > > To break this deadlock and allow the mode-switch registration to occur, > purge all the usb-c-connector nodes' absent suppliers. This eliminates > the connector as a supplier for a switch and allows it to be probed. ... > + /* > + * OF graph may have set up some device links with switches, since > connectors have their > + * own compatible. Purge these to avoid a deadlock in switch probe (the > switch mistakenly > + * assumes the connector is a supplier). > + */ A bit too long lines... > + if (dev->of_node) Why do you need this check? > + device_for_each_child_node(dev, fwnode) > + fw_devlink_purge_absent_suppliers(fwnode); -- With Best Regards, Andy Shevchenko
Re: [PATCH v6 7/7] drm/bridge: it6505: Register Type C mode switches
On Thu, Nov 24, 2022 at 06:20:56PM +0800, Pin-yen Lin wrote:
> Register USB Type-C mode switches when the "mode-switch" property and
> relevant port are available in Device Tree. Configure the "lane_swap"
> state based on the entered alternate mode for a specific Type-C
> connector, which ends up updating the lane swap registers of the it6505
> chip.
...
> config DRM_ITE_IT6505
> tristate "ITE IT6505 DisplayPort bridge"
> depends on OF
> + depends on TYPEC || TYPEC=n
> select DRM_DISPLAY_DP_HELPER
> select DRM_DISPLAY_HDCP_HELPER
> select DRM_DISPLAY_HELPER
Something went wrong with the indentation. Perhaps you need to fix it first.
...
> #include
> #include
> #include
> +#include
Make it ordered?
...
> +struct it6505_port_data {
> + bool dp_connected;
Perhaps make it last?
> + struct typec_mux_dev *typec_mux;
> + struct it6505 *it6505;
> +};
...
> +static void it6505_typec_ports_update(struct it6505 *it6505)
> +{
> + usleep_range(3000, 4000);
> +
> + if (it6505->typec_ports[0].dp_connected &&
> it6505->typec_ports[1].dp_connected)
> + /* Both ports available, do nothing to retain the current one.
> */
> + return;
> + else if (it6505->typec_ports[0].dp_connected)
> + it6505->lane_swap = false;
> + else if (it6505->typec_ports[1].dp_connected)
> + it6505->lane_swap = true;
> +
> + usleep_range(3000, 4000);
> +}
As per previous patch comments.
Also, comment out these long sleeps. Why they are needed.
...
> + int ret = pm_runtime_get_sync(dev);
> +
> + /*
> + * On system resume, mux_set can be triggered before
> + * pm_runtime_force_resume re-enables runtime power management.
We refer to the functions as func().
> + * Handling the error here to make sure the bridge is powered
> on.
> + */
> + if (ret < 0)
> + it6505_poweron(it6505);
This seems needed a bit more of explanation, esp. why you leave PM runtime
reference count bumped up.
...
> + num_lanes = drm_of_get_data_lanes_count(node, 0, 2);
> + if (num_lanes <= 0) {
> + dev_err(dev, "Error on getting data lanes count: %d\n",
> + num_lanes);
> + return num_lanes;
> + }
> +
> + ret = of_property_read_u32_array(node, "data-lanes", dp_lanes,
> num_lanes);
> + if (ret) {
> + dev_err(dev, "Failed to read the data-lanes variable: %d\n",
> + ret);
> + return ret;
> + }
> +
> + for (i = 0; i < num_lanes; i++) {
> + if (port_num != -1 && port_num != dp_lanes[i] / 2) {
> + dev_err(dev, "Invalid data lane numbers\n");
> + return -EINVAL;
> + }
As per previous patch comments.
> + port_num = dp_lanes[i] / 2;
> + }
The above seems like tons of duplicating code that drivers need to implement.
Can we shrink that burden by adding some library functions?
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v6 5/7] drm/bridge: anx7625: Register Type C mode switches
On Thu, Nov 24, 2022 at 06:20:54PM +0800, Pin-yen Lin wrote:
> Register USB Type-C mode switches when the "mode-switch" property and
> relevant port are available in Device Tree. Configure the crosspoint
> switch based on the entered alternate mode for a specific Type-C
> connector.
...
> +static void anx7625_typec_two_ports_update(struct anx7625_data *ctx)
> +{
> + if (ctx->typec_ports[0].dp_connected &&
> ctx->typec_ports[1].dp_connected)
> + /* Both ports available, do nothing to retain the current one.
> */
> + return;
> + else if (ctx->typec_ports[0].dp_connected)
This 'else' is redundant. I would rewrite above as
/* Check if both ports available and do nothing to retain the current
one */
if (ctx->typec_ports[0].dp_connected &&
ctx->typec_ports[1].dp_connected)
return;
if (ctx->typec_ports[0].dp_connected)
> + anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_NORMAL);
> + else if (ctx->typec_ports[1].dp_connected)
> + anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_REVERSE);
> +}
...
> + data->dp_connected = (state->alt && state->alt->svid ==
> USB_TYPEC_DP_SID &&
> + state->alt->mode == USB_TYPEC_DP_MODE);
Parentheses are not needed.
...
> + /*
> + * <0 1> refers to SSRX1/SSTX1, and <2 3> refers to SSRX2/SSTX2.
> + */
> + for (i = 0; i < num_lanes; i++) {
> + if (port_num != -1 && port_num != dp_lanes[i] / 2) {
> + dev_err(dev, "Invalid data lane numbers\n");
> + return -EINVAL;
> + }
According to Rob Linux must not validate device tree. If you need it, use
proper YAML schema.
> + port_num = dp_lanes[i] / 2;
> + }
...
> + if (!ctx->num_typec_switches) {
> + dev_warn(dev, "No Type-C switches node found\n");
> + return ret;
Why not to return 0 explicitly?
> + }
...
> + ctx->typec_ports = devm_kcalloc(
Broken indentation.
> + dev, ctx->num_typec_switches, sizeof(struct anx7625_port_data),
> + GFP_KERNEL);
> + if (!ctx->typec_ports)
> + return -ENOMEM;
...
> +struct anx7625_port_data {
> + bool dp_connected;
You can save some bytes on some architectures if move this to be last field.
> + struct typec_mux_dev *typec_mux;
> + struct anx7625_data *ctx;
> +};
--
With Best Regards,
Andy Shevchenko
Re: [PATCH] drm/bridge: ti-sn65dsi83: Fix delay after reset deassert to match spec
Applied to drm-misc-next.
Re: [PATCH] dma-buf: Fix possible UAF in dma_buf_export
Some tips: Before we call the dma_buf_stats_setup(), we have to finish creating the file, otherwise dma_buf_stats_setup() will return -EINVAL, maybe we need to think about this when making a new patch. Hope these tips are useful, thanks! On 2022/11/24 13:56, Charan Teja Kalla wrote: Thanks T.J and Christian for the inputs. On 11/19/2022 7:00 PM, Christian König wrote: Yes, exactly that's the idea. The only alternatives I can see would be to either move allocating the file and so completing the dma_buf initialization last again or just ignore errors from sysfs. > If we still want to avoid calling dmabuf->ops->release(dmabuf) in > dma_buf_release like the comment says I guess we could use sysfs_entry > and ERR_PTR to flag that, otherwise it looks like we'd need a bit > somewhere. No, this should be dropped as far as I can see. The sysfs cleanup code looks like it can handle not initialized kobj pointers. Yeah there is also the null check in dma_buf_stats_teardown() that would prevent it from running, but I understood the comment to be referring to the release() dma_buf_ops call into the exporter which comes right after the teardown call. That looks like it's preventing the fput task work calling back into the exporter after the exporter already got an error from dma_buf_export(). Otherwise the exporter sees a release() for a buffer that it doesn't know about / thinks shouldn't exist. So I could imagine an exporter trying to double free: once for the failed dma_buf_export() call, and again when the release() op is called later. Oh, very good point as well. Yeah, then creating the file should probably come last. @Gaosheng: Could you please make these changes or you let me to do? Regards, Christian. .
Re: [Intel-gfx] [PATCH v2 2/4] drm/i915: Introduce guard pages to i915_vma
On 23/11/2022 18:54, Andi Shyti wrote:
Hi Tvrtko,
[...]
@@ -768,6 +768,9 @@ i915_vma_insert(struct i915_vma *vma, struct
i915_gem_ww_ctx *ww,
GEM_BUG_ON(!IS_ALIGNED(alignment, I915_GTT_MIN_ALIGNMENT));
GEM_BUG_ON(!is_power_of_2(alignment));
+ guard = vma->guard; /* retain guard across rebinds */
+ guard = ALIGN(guard, alignment);
Why does guard area needs the same alignment as the requested mapping? What
about the fact on 32-bit builds guard is 32-bit and alignment u64?
I guess this just to round up/down guard to something, not
necessarily to that alignment.
Shall I remove it?
Don't know, initially I thought it maybe needs a comment on what's it
doing and why. If it is about aligning to "something" then should it be
I915_GTT_MIN_ALIGNMENT?
@@ -777,6 +780,7 @@ i915_vma_insert(struct i915_vma *vma, struct
i915_gem_ww_ctx *ww,
if (flags & PIN_ZONE_4G)
end = min_t(u64, end, (1ULL << 32) - I915_GTT_PAGE_SIZE);
GEM_BUG_ON(!IS_ALIGNED(end, I915_GTT_PAGE_SIZE));
+ GEM_BUG_ON(2 * guard > end);
End is the size of relevant VA area at this point so what and why is this
checking?
I think because we want to make sure the padding is at least not
bigger that the size. What is actually wrong with this.
Same as above - if there is subtle special meaning please add a comment.
Otherwise, for the whole object and not just the guards, it is covered by:
+ if (size > end - 2 * guard) {
I don't follow what is the point on only checking the guards.
[...]
@@ -855,6 +869,7 @@ i915_vma_insert(struct i915_vma *vma, struct
i915_gem_ww_ctx *ww,
GEM_BUG_ON(!i915_gem_valid_gtt_space(vma, color));
list_move_tail(&vma->vm_link, &vma->vm->bound_list);
+ vma->guard = guard;
unsigned long into u32 - what guarantees no truncation?
we are missing here this part above:
guard = vma->guard; /* retain guard across rebinds */
if (flags & PIN_OFFSET_GUARD) {
GEM_BUG_ON(overflows_type(flags & PIN_OFFSET_MASK, u32));
guard = max_t(u32, guard, flags & PIN_OFFSET_MASK);
}
that should make sure that we fit into 32 bits.
Hm okay. I guess the u64 alignment and that "guard = ALIGN(guard,
alignment);" is what is bothering me to begin with. In other words with
that there is a chance to overflow vma->guard with a small guard and
large alignment.
[...]
@@ -197,14 +197,15 @@ struct i915_vma {
struct i915_fence_reg *fence;
u64 size;
- u64 display_alignment;
struct i915_page_sizes page_sizes;
/* mmap-offset associated with fencing for this vma */
struct i915_mmap_offset *mmo;
+ u32 guard; /* padding allocated around vma->pages within the node */
u32 fence_size;
u32 fence_alignment;
+ u32 display_alignment;
u64 -> u32 for display_alignment looks unrelated change.
./display/intel_fb_pin.c: vma->display_alignment = max_t(u64,
vma->display_alignment, alignment);
./gem/i915_gem_domain.c:vma->display_alignment = max_t(u64,
vma->display_alignment, alignment);
These two sites need to be changed not to use u64.
Do this part in a separate patch?
Right! will remove it.
Okay, to be clear, refactoring of vma->display_alignemnt to be u32 as a
separate patch in the series. Thanks!
Regards,
Tvrtko
/**
* Count of the number of times this vma has been opened by different
Regards,
Thanks,
Andi
Tvrtko
Re: [PATCH v10 00/19] drm: Analog TV Improvements
On Mon, Nov 21, 2022 at 03:51:26PM +0100, Daniel Vetter wrote: > On Thu, Nov 17, 2022 at 10:28:43AM +0100, Maxime Ripard wrote: > > Hi, > > > > Here's a series aiming at improving the command line named modes support, > > and more importantly how we deal with all the analog TV variants. > > > > The named modes support were initially introduced to allow to specify the > > analog TV mode to be used. > > > > However, this was causing multiple issues: > > > > * The mode name parsed on the command line was passed directly to the > > driver, which had to figure out which mode it was suppose to match; > > > > * Figuring that out wasn't really easy, since the video= argument or what > > the userspace might not even have a name in the first place, but > > instead could have passed a mode with the same timings; > > > > * The fallback to matching on the timings was mostly working as long as > > we were supporting one 525 lines (most likely NSTC) and one 625 lines > > (PAL), but couldn't differentiate between two modes with the same > > timings (NTSC vs PAL-M vs NSTC-J for example); > > > > * There was also some overlap with the tv mode property registered by > > drm_mode_create_tv_properties(), but named modes weren't interacting > > with that property at all. > > > > * Even though that property was generic, its possible values were > > specific to each drivers, which made some generic support difficult. > > > > Thus, I chose to tackle in multiple steps: > > > > * A new TV mode property was introduced, with generic values, each driver > > reporting through a bitmask what standard it supports to the userspace; > > > > * This option was added to the command line parsing code to be able to > > specify it on the kernel command line, and new atomic_check and reset > > helpers were created to integrate properly into atomic KMS; > > > > * The named mode parsing code is now creating a proper display mode for > > the given named mode, and the TV standard will thus be part of the > > connector state; > > > > * Two drivers were converted and tested for now (vc4 and sun4i), with > > some backward compatibility code to translate the old TV mode to the > > new TV mode; > > > > Unit tests were created along the way. > > > > One can switch from NTSC to PAL now using (on vc4) > > > > modetest -M vc4 -s 53:720x480i -w 53:'TV mode':1 # NTSC > > modetest -M vc4 -s 53:720x576i -w 53:'TV mode':4 # PAL > > > > Let me know what you think, > > Maxime > > Maxime asked me to drop an Ack-in-principle on this, and I'm not sure I > have any useful input here with my utter lack of understanding for TV > things (I never even had one in my entire life, that's how much I don't > care). But it seems to check all the design boxes around solving annoying > uapi/kms-config issues properly, so > > Acked-in-principle-or-something-like-that-by: Daniel Vetter > Thanks! I jumped the gun a bit too fast and forgot to amend the TV property commit message before pushing it out. For the record though, that property is usable through xrandr, xorg.conf or any equivalent compositor mechanism Maxime signature.asc Description: PGP signature
Re: [PATCH] dma-buf: Fix possible UAF in dma_buf_export
Thanks T.J and Christian, thanks everyone for taking time to review this patch. Charan, actually I don't have a good patch to to fix it, if you can submit a new patch to solve it, please feel free to do it. By the way, I'd appreciate it if you could send to me the new patch when you submit it. Thanks again! Gaosheng. On 2022/11/24 13:56, Charan Teja Kalla wrote: Thanks T.J and Christian for the inputs. On 11/19/2022 7:00 PM, Christian König wrote: Yes, exactly that's the idea. The only alternatives I can see would be to either move allocating the file and so completing the dma_buf initialization last again or just ignore errors from sysfs. > If we still want to avoid calling dmabuf->ops->release(dmabuf) in > dma_buf_release like the comment says I guess we could use sysfs_entry > and ERR_PTR to flag that, otherwise it looks like we'd need a bit > somewhere. No, this should be dropped as far as I can see. The sysfs cleanup code looks like it can handle not initialized kobj pointers. Yeah there is also the null check in dma_buf_stats_teardown() that would prevent it from running, but I understood the comment to be referring to the release() dma_buf_ops call into the exporter which comes right after the teardown call. That looks like it's preventing the fput task work calling back into the exporter after the exporter already got an error from dma_buf_export(). Otherwise the exporter sees a release() for a buffer that it doesn't know about / thinks shouldn't exist. So I could imagine an exporter trying to double free: once for the failed dma_buf_export() call, and again when the release() op is called later. Oh, very good point as well. Yeah, then creating the file should probably come last. @Gaosheng: Could you please make these changes or you let me to do? Regards, Christian. .
[PATCH linux-next] dma-buf: use strscpy() to instead of strlcpy()
From: Xu Panda The implementation of strscpy() is more robust and safer. That's now the recommended way to copy NUL terminated strings. Signed-off-by: Xu Panda Signed-off-by: Yang Yang --- drivers/dma-buf/dma-buf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index b6c36914e7c6..485cf4f3431e 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -51,7 +51,7 @@ static char *dmabuffs_dname(struct dentry *dentry, char *buffer, int buflen) dmabuf = dentry->d_fsdata; spin_lock(&dmabuf->name_lock); if (dmabuf->name) - ret = strlcpy(name, dmabuf->name, DMA_BUF_NAME_LEN); + ret = strscpy(name, dmabuf->name, DMA_BUF_NAME_LEN); spin_unlock(&dmabuf->name_lock); return dynamic_dname(buffer, buflen, "/%s:%s", -- 2.15.2
Re: [PATCH resend v2] drm/fourcc: Add missing big-endian XRGB1555 and RGB565 formats
Hi, > > Supporting 16 bpp in the driver wouldn't be that much of a problem, but > > processing the framebuffer on the host side when emulating a big endian > > guest on a little endian host is painful. I think I can't ask pixman to > > do a conversation from DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN to > > DRM_FORMAT_XRGB on a little endian machine. > > Indeed. But you can do a quick 16-bit byteswap, and convert from > DRM_FORMAT_RGB565 to DRM_FORMAT_XRGB? Sure doable, but it's an extra step in a rarely tested code path ... > BTW, does pixman support converting DRM_FORMAT_RGB565 to > DRM_FORMAT_XRGB on a big-endian machine? I don't think so. When you can get the color bits with shifting and masking pixman is happy. For rgb565 (and xrgb1555) that works only on native byte order. take care, Gerd
Re: [PATCH] drm/dma-helpers: Don't change vma flags
On 2022-11-23 17:28, Daniel Vetter wrote:
This code was added in b65e64f7ccd4 ("drm/cma: Use
dma_mmap_writecombine() to mmap buffer"), but does not explain why
it's needed.
It should be entirely unnecessary, because remap_pfn_range(), which is
what the various dma_mmap functiosn are built on top of, does already
unconditionally adjust the vma flags:
Not all dma_mmap_*() implementations use remap_pfn_range() though. For
instance on ARM where one set of DMA ops uses vm_map_pages(), but AFAICS
not all the relevant drivers would set VM_MIXEDMAP to prevent reaching
the BUG_ON(vma->vm_flags & VM_PFNMAP) in there.
Robin.
https://elixir.bootlin.com/linux/v6.1-rc6/source/mm/memory.c#L2518
More importantly, it does uncondtionally set VM_PFNMAP, so clearing
that does not make much sense.
Patch motived by discussions around enforcing VM_PFNMAP semantics for
all dma-buf users, where Thomas asked why dma helpers will work with
that dma_buf_mmap() contract.
References:
https://lore.kernel.org/dri-devel/5c3c8d4f-2c06-9210-b00a-4d0ff6f6f...@suse.de/
Cc: Laurent Pinchart
Cc: Dave Airlie
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: Sumit Semwal
Cc: "Christian König"
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_gem_dma_helper.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_dma_helper.c
b/drivers/gpu/drm/drm_gem_dma_helper.c
index 1e658c448366..637a5cc62457 100644
--- a/drivers/gpu/drm/drm_gem_dma_helper.c
+++ b/drivers/gpu/drm/drm_gem_dma_helper.c
@@ -525,13 +525,10 @@ int drm_gem_dma_mmap(struct drm_gem_dma_object *dma_obj,
struct vm_area_struct *
int ret;
/*
-* Clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and set the
-* vm_pgoff (used as a fake buffer offset by DRM) to 0 as we want to map
-* the whole buffer.
+* Set the vm_pgoff (used as a fake buffer offset by DRM) to 0 as we
+* want to map the whole buffer.
*/
vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node);
- vma->vm_flags &= ~VM_PFNMAP;
- vma->vm_flags |= VM_DONTEXPAND;
if (dma_obj->map_noncoherent) {
vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
Re: [PATCH] drm/bridge: ti-sn65dsi83: Fix delay after reset deassert to match spec
On 24.11.22 12:00, Alexander Stein wrote:
> Am Dienstag, 22. November 2022, 09:12:18 CET schrieb Frieder Schrempf:
>> From: Frieder Schrempf
>>
>> The datasheet specifies a delay of 10 milliseconds, but the current
>> driver only waits for 1 ms. Fix this to make sure the initialization
>> sequence meets the spec.
>>
>> Fixes: ceb515ba29ba ("drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and
>> SN65DSI84 driver") Signed-off-by: Frieder Schrempf
>>
>> ---
>> drivers/gpu/drm/bridge/ti-sn65dsi83.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
>> b/drivers/gpu/drm/bridge/ti-sn65dsi83.c index 7ba9467fff12..047c14ddbbf1
>> 100644
>> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
>> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
>> @@ -346,7 +346,7 @@ static void sn65dsi83_atomic_enable(struct drm_bridge
>> *bridge,
>>
>> /* Deassert reset */
>> gpiod_set_value_cansleep(ctx->enable_gpio, 1);
>> -usleep_range(1000, 1100);
>> +usleep_range(1, 11000);
>>
>> /* Get the LVDS format from the bridge state. */
>> bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
>
> How about using fsleep?
>
> Either way:
> Reviewed-by: Alexander Stein
Thanks for the review! I didn't know about fsleep. Anyway this would
probably be a separate change as the driver currently uses usleep
everywhere else. But I will keep it in mind for the future.
Re: [PATCH] drm/bridge: ti-sn65dsi83: Fix delay after reset deassert to match spec
Am Dienstag, 22. November 2022, 09:12:18 CET schrieb Frieder Schrempf:
> From: Frieder Schrempf
>
> The datasheet specifies a delay of 10 milliseconds, but the current
> driver only waits for 1 ms. Fix this to make sure the initialization
> sequence meets the spec.
>
> Fixes: ceb515ba29ba ("drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and
> SN65DSI84 driver") Signed-off-by: Frieder Schrempf
>
> ---
> drivers/gpu/drm/bridge/ti-sn65dsi83.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> b/drivers/gpu/drm/bridge/ti-sn65dsi83.c index 7ba9467fff12..047c14ddbbf1
> 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> @@ -346,7 +346,7 @@ static void sn65dsi83_atomic_enable(struct drm_bridge
> *bridge,
>
> /* Deassert reset */
> gpiod_set_value_cansleep(ctx->enable_gpio, 1);
> - usleep_range(1000, 1100);
> + usleep_range(1, 11000);
>
> /* Get the LVDS format from the bridge state. */
> bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
How about using fsleep?
Either way:
Reviewed-by: Alexander Stein
Re: [PATCH] drm/amdgpu: add mb for si
On 11/24/2022 4:11 PM, Lazar, Lijo wrote: On 11/24/2022 3:34 PM, Quan, Evan wrote: [AMD Official Use Only - General] Could the attached patch help? Evan -Original Message- From: amd-gfx On Behalf Of ??? Sent: Friday, November 18, 2022 5:25 PM To: Michel Dänzer ; Koenig, Christian ; Deucher, Alexander Cc: amd-...@lists.freedesktop.org; Pan, Xinhui ; linux-ker...@vger.kernel.org; dri-devel@lists.freedesktop.org Subject: Re: [PATCH] drm/amdgpu: add mb for si 在 2022/11/18 17:18, Michel Dänzer 写道: On 11/18/22 09:01, Christian König wrote: Am 18.11.22 um 08:48 schrieb Zhenneng Li: During reboot test on arm64 platform, it may failure on boot, so add this mb in smc. The error message are as follows: [ 6.996395][ 7] [ T295] [drm:amdgpu_device_ip_late_init [amdgpu]] *ERROR* late_init of IP block failed -22 [ 7.006919][ 7] [ T295] amdgpu :04:00.0: The issue is happening in late_init() which eventually does ret = si_thermal_enable_alert(adev, false); Just before this, si_thermal_start_thermal_controller is called in hw_init and that enables thermal alert. Maybe the issue is with enable/disable of thermal alerts in quick succession. Adding a delay inside si_thermal_start_thermal_controller might help. On a second look, temperature range is already set as part of si_thermal_start_thermal_controller in hw_init https://elixir.bootlin.com/linux/v6.1-rc6/source/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c#L6780 There is no need to set it again here - https://elixir.bootlin.com/linux/v6.1-rc6/source/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c#L7635 I think it is safe to remove the call from late_init altogether. Alex/Evan? Thanks, Lijo Thanks, Lijo amdgpu_device_ip_late_init failed [ 7.014224][ 7] [ T295] amdgpu :04:00.0: Fatal error during GPU init Memory barries are not supposed to be sprinkled around like this, you need to give a detailed explanation why this is necessary. Regards, Christian. Signed-off-by: Zhenneng Li --- drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c b/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c index 8f994ffa9cd1..c7656f22278d 100644 --- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c @@ -155,6 +155,8 @@ bool amdgpu_si_is_smc_running(struct amdgpu_device *adev) u32 rst = RREG32_SMC(SMC_SYSCON_RESET_CNTL); u32 clk = RREG32_SMC(SMC_SYSCON_CLOCK_CNTL_0); + mb(); + if (!(rst & RST_REG) && !(clk & CK_DISABLE)) return true; In particular, it makes no sense in this specific place, since it cannot directly affect the values of rst & clk. I thinks so too. But when I do reboot test using nine desktop machines, there maybe report this error on one or two machines after Hundreds of times or Thousands of times reboot test, at the beginning, I use msleep() instead of mb(), these two methods are all works, but I don't know what is the root case. I use this method on other verdor's oland card, this error message are reported again. What could be the root reason? test environmen: graphics card: OLAND 0x1002:0x6611 0x1642:0x1869 0x87 driver: amdgpu os: ubuntu 2004 platform: arm64 kernel: 5.4.18
Re: [PATCH] drm/amdgpu: add mb for si
On 11/24/2022 3:34 PM, Quan, Evan wrote: [AMD Official Use Only - General] Could the attached patch help? Evan -Original Message- From: amd-gfx On Behalf Of ??? Sent: Friday, November 18, 2022 5:25 PM To: Michel Dänzer ; Koenig, Christian ; Deucher, Alexander Cc: amd-...@lists.freedesktop.org; Pan, Xinhui ; linux-ker...@vger.kernel.org; dri-devel@lists.freedesktop.org Subject: Re: [PATCH] drm/amdgpu: add mb for si 在 2022/11/18 17:18, Michel Dänzer 写道: On 11/18/22 09:01, Christian König wrote: Am 18.11.22 um 08:48 schrieb Zhenneng Li: During reboot test on arm64 platform, it may failure on boot, so add this mb in smc. The error message are as follows: [6.996395][ 7] [ T295] [drm:amdgpu_device_ip_late_init [amdgpu]] *ERROR* late_init of IP block failed -22 [ 7.006919][ 7] [ T295] amdgpu :04:00.0: The issue is happening in late_init() which eventually does ret = si_thermal_enable_alert(adev, false); Just before this, si_thermal_start_thermal_controller is called in hw_init and that enables thermal alert. Maybe the issue is with enable/disable of thermal alerts in quick succession. Adding a delay inside si_thermal_start_thermal_controller might help. Thanks, Lijo amdgpu_device_ip_late_init failed [7.014224][ 7] [ T295] amdgpu :04:00.0: Fatal error during GPU init Memory barries are not supposed to be sprinkled around like this, you need to give a detailed explanation why this is necessary. Regards, Christian. Signed-off-by: Zhenneng Li --- drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c b/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c index 8f994ffa9cd1..c7656f22278d 100644 --- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c @@ -155,6 +155,8 @@ bool amdgpu_si_is_smc_running(struct amdgpu_device *adev) u32 rst = RREG32_SMC(SMC_SYSCON_RESET_CNTL); u32 clk = RREG32_SMC(SMC_SYSCON_CLOCK_CNTL_0); +mb(); + if (!(rst & RST_REG) && !(clk & CK_DISABLE)) return true; In particular, it makes no sense in this specific place, since it cannot directly affect the values of rst & clk. I thinks so too. But when I do reboot test using nine desktop machines, there maybe report this error on one or two machines after Hundreds of times or Thousands of times reboot test, at the beginning, I use msleep() instead of mb(), these two methods are all works, but I don't know what is the root case. I use this method on other verdor's oland card, this error message are reported again. What could be the root reason? test environmen: graphics card: OLAND 0x1002:0x6611 0x1642:0x1869 0x87 driver: amdgpu os: ubuntu 2004 platform: arm64 kernel: 5.4.18
Re: [PATCH 1/3] drm/ttm: remove ttm_bo_(un)lock_delayed_workqueue
On Thu, 24 Nov 2022 at 10:03, Christian König wrote: > > Those functions never worked correctly since it is still perfectly > possible that a buffer object is released and the background worker > restarted even after calling them. > > Signed-off-by: Christian König I know you usually do, but just a friendly reminder to Cc: intel-gfx on the next revision or before merging, just so our CI can give the series a quick test. Thanks.
[PULL] drm-misc-fixes
Hey Daniel and Dae, Not much here, a few fixes to dma-fence handling and a fix to amdgpu and logo. Enjoy! Maarten Lankhorst drm-misc-fixes-2022-11-24: drm-misc-fixes for v6.1-rc7: - Another amdgpu gang submit fix. - Use dma_fence_unwrap_for_each when importing sync files. - Fix race in dma_heap_add(). - Fix use of uninitialized memory in logo. The following changes since commit 5954acbacbd1946b96ce8ee799d309cb0cd3cb9d: drm/display: Don't assume dual mode adaptors support i2c sub-addressing (2022-11-15 23:31:02 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2022-11-24 for you to fetch changes up to a6a00d7e8ffd78d1cdb7a43f1278f081038c638f: fbcon: Use kzalloc() in fbcon_prepare_logo() (2022-11-22 15:48:02 +0100) drm-misc-fixes for v6.1-rc7: - Another amdgpu gang submit fix. - Use dma_fence_unwrap_for_each when importing sync files. - Fix race in dma_heap_add(). - Fix use of uninitialized memory in logo. Christian König (1): drm/amdgpu: handle gang submit before VMID Dawei Li (1): dma-buf: fix racing conflict of dma_heap_add() Jason Ekstrand (1): dma-buf: Use dma_fence_unwrap_for_each when importing fences Tetsuo Handa (1): fbcon: Use kzalloc() in fbcon_prepare_logo() drivers/dma-buf/dma-buf.c | 23 +-- drivers/dma-buf/dma-heap.c | 28 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 6 +++--- drivers/video/fbdev/core/fbcon.c| 2 +- 4 files changed, 36 insertions(+), 23 deletions(-)
[PATCH v6 6/7] dt/bindings: drm/bridge: it6505: Add mode-switch support
ITE IT6505 can be used in systems to switch the DP traffic between
two downstreams, which can be USB Type-C DisplayPort alternate mode
lane or regular DisplayPort output ports.
Update the binding to accommodate this usage by introducing a
data-lanes and a mode-switch property on endpoints.
Signed-off-by: Pin-yen Lin
---
Changes in v6:
- Remove switches node and use endpoints and data-lanes property to
describe the connections.
.../bindings/display/bridge/ite,it6505.yaml | 94 ++-
1 file changed, 90 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
index 833d11b2303a..b4b9881c7759 100644
--- a/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml
@@ -52,9 +52,53 @@ properties:
maxItems: 1
description: extcon specifier for the Power Delivery
- port:
-$ref: /schemas/graph.yaml#/properties/port
-description: A port node pointing to DPI host port node
+ data-lanes:
+maxItems: 1
+description: restrict the dp output data-lanes with value of 1-4
+
+ max-pixel-clock-khz:
+maxItems: 1
+description: restrict max pixel clock
+
+ ports:
+$ref: /schemas/graph.yaml#/properties/ports
+
+properties:
+ port@0:
+$ref: /schemas/graph.yaml#/$defs/port-base
+unevaluatedProperties: false
+description: A port node pointing to DPI host port node
+
+ port@1:
+$ref: /schemas/graph.yaml#/properties/port-base
+description:
+ Video port for panel or connector.
+
+patternProperties:
+ "^endpoint@[01]$":
+$ref: /schemas/media/video-interfaces.yaml#
+type: object
+unevaluatedProperties: false
+
+properties:
+ reg:
+maxItems: 1
+
+ remote-endpoint: true
+
+ data-lanes:
+minItems: 1
+uniqueItems: true
+items:
+ - enum: [ 0, 1, 2, 3]
+
+ mode-switch:
+type: boolean
+description: Register this node as a Type-C mode switch or not.
+
+ required:
+- reg
+ - remote-endpoint
required:
- compatible
@@ -62,7 +106,7 @@ required:
- pwr18-supply
- interrupts
- reset-gpios
- - extcon
+ - ports
additionalProperties: false
@@ -92,3 +136,45 @@ examples:
};
};
};
+ - |
+#include
+
+&i2c3 {
+clock-frequency = <10>;
+
+it6505dptx: it6505dptx@5c {
+compatible = "ite,it6505";
+interrupts = <8 IRQ_TYPE_LEVEL_LOW 8 0>;
+reg = <0x5c>;
+pinctrl-names = "default";
+pinctrl-0 = <&it6505_pins>;
+ovdd-supply = <&mt6366_vsim2_reg>;
+pwr18-supply = <&pp1800_dpbrdg_dx>;
+reset-gpios = <&pio 177 0>;
+hpd-gpios = <&pio 10 0>;
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+port@0 {
+reg = <0>;
+it6505_in: endpoint {
+remote-endpoint = <&dpi_out>;
+};
+};
+port@1 {
+reg = <1>;
+ite_typec0: endpoint@0 {
+mode-switch;
+data-lanes = <0 1>;
+remote-endpoint = <&typec_port0>;
+};
+ite_typec1: endpoint@1 {
+mode-switch;
+data-lanes = <2 3>;
+remote-endpoint = <&typec_port1>;
+};
+};
+};
+};
+};
--
2.38.1.584.g0f3c55d4c2-goog
[PATCH v6 7/7] drm/bridge: it6505: Register Type C mode switches
Register USB Type-C mode switches when the "mode-switch" property and
relevant port are available in Device Tree. Configure the "lane_swap"
state based on the entered alternate mode for a specific Type-C
connector, which ends up updating the lane swap registers of the it6505
chip.
Signed-off-by: Pin-yen Lin
---
Changes in v6:
- Changed it6505_typec_mux_set callback function to accommodate with
the latest drm-misc patches
- Changed the driver implementation to accommodate with the new binding
- Squashed to a single patch
drivers/gpu/drm/bridge/Kconfig | 1 +
drivers/gpu/drm/bridge/ite-it6505.c | 205 +++-
2 files changed, 202 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 57946d80b02d..9b1240ef3981 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -87,6 +87,7 @@ config DRM_FSL_LDB
config DRM_ITE_IT6505
tristate "ITE IT6505 DisplayPort bridge"
depends on OF
+ depends on TYPEC || TYPEC=n
select DRM_DISPLAY_DP_HELPER
select DRM_DISPLAY_HDCP_HELPER
select DRM_DISPLAY_HELPER
diff --git a/drivers/gpu/drm/bridge/ite-it6505.c
b/drivers/gpu/drm/bridge/ite-it6505.c
index 21a9b8422bda..37e2bd8d8d79 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -17,6 +17,8 @@
#include
#include
#include
+#include
+#include
#include
#include
@@ -30,6 +32,7 @@
#include
#include
#include
+#include
#include
@@ -402,6 +405,12 @@ struct debugfs_entries {
const struct file_operations *fops;
};
+struct it6505_port_data {
+ bool dp_connected;
+ struct typec_mux_dev *typec_mux;
+ struct it6505 *it6505;
+};
+
struct it6505 {
struct drm_dp_aux aux;
struct drm_bridge bridge;
@@ -454,6 +463,8 @@ struct it6505 {
struct delayed_work delayed_audio;
struct it6505_audio_data audio;
struct dentry *debugfs;
+ int num_typec_switches;
+ struct it6505_port_data *typec_ports;
/* it6505 driver hold option */
bool enable_drv_hold;
@@ -3265,13 +3276,185 @@ static void it6505_shutdown(struct i2c_client *client)
it6505_lane_off(it6505);
}
+static void it6505_typec_ports_update(struct it6505 *it6505)
+{
+ usleep_range(3000, 4000);
+
+ if (it6505->typec_ports[0].dp_connected &&
it6505->typec_ports[1].dp_connected)
+ /* Both ports available, do nothing to retain the current one.
*/
+ return;
+ else if (it6505->typec_ports[0].dp_connected)
+ it6505->lane_swap = false;
+ else if (it6505->typec_ports[1].dp_connected)
+ it6505->lane_swap = true;
+
+ usleep_range(3000, 4000);
+}
+
+static int it6505_typec_mux_set(struct typec_mux_dev *mux,
+ struct typec_mux_state *state)
+{
+ struct it6505_port_data *data = typec_mux_get_drvdata(mux);
+ struct it6505 *it6505 = data->it6505;
+ struct device *dev = &it6505->client->dev;
+ bool old_dp_connected, new_dp_connected;
+
+ if (it6505->num_typec_switches == 1)
+ return 0;
+
+ mutex_lock(&it6505->extcon_lock);
+
+ old_dp_connected = it6505->typec_ports[0].dp_connected ||
+ it6505->typec_ports[1].dp_connected;
+
+ data->dp_connected = (state->alt && state->alt->svid ==
USB_TYPEC_DP_SID &&
+ state->alt->mode == USB_TYPEC_DP_MODE);
+
+ dev_dbg(dev, "mux_set dp_connected: c0=%d, c1=%d\n",
+ it6505->typec_ports[0].dp_connected,
it6505->typec_ports[1].dp_connected);
+
+ new_dp_connected = it6505->typec_ports[0].dp_connected ||
+ it6505->typec_ports[1].dp_connected;
+
+ if (it6505->enable_drv_hold) {
+ dev_dbg(dev, "enable driver hold\n");
+ goto unlock;
+ }
+
+ it6505_typec_ports_update(it6505);
+
+ if (!old_dp_connected && new_dp_connected) {
+ int ret = pm_runtime_get_sync(dev);
+
+ /*
+* On system resume, mux_set can be triggered before
+* pm_runtime_force_resume re-enables runtime power management.
+* Handling the error here to make sure the bridge is powered
on.
+*/
+ if (ret < 0)
+ it6505_poweron(it6505);
+
+ complete_all(&it6505->extcon_completion);
+ }
+
+ if (old_dp_connected && !new_dp_connected) {
+ reinit_completion(&it6505->extcon_completion);
+ pm_runtime_put_sync(dev);
+ if (it6505->bridge.dev)
+ drm_helper_hpd_irq_event(it6505->bridge.dev);
+ memset(it6505->dpcd, 0, sizeof(it6505->dpcd));
+ }
+
+unlock:
+ mutex_unlock(&it6505->extcon_lock);
+ return 0;
+}
+
+static int
[PATCH v6 4/7] drm/bridge: anx7625: Check for Type-C during panel registration
The output port endpoints can be connected to USB-C connectors.
Running drm_of_find_panel_or_bridge() with such endpoints leads to
a continuous return value of -EPROBE_DEFER, even though there is
no panel present.
To avoid this, check for the existence of a "mode-switch" property in
the port endpoint, and skip panel registration completely if so.
Signed-off-by: Pin-yen Lin
---
Changes in v6:
- New in v6
drivers/gpu/drm/bridge/analogix/anx7625.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index b0ff1ecb80a5..b9e3d6ad8846 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -1650,7 +1650,7 @@ static int anx7625_get_swing_setting(struct device *dev,
static int anx7625_parse_dt(struct device *dev,
struct anx7625_platform_data *pdata)
{
- struct device_node *np = dev->of_node, *ep0;
+ struct device_node *np = dev->of_node, *ep0, *sw;
int bus_type, mipi_lanes;
anx7625_get_swing_setting(dev, pdata);
@@ -1689,6 +1689,17 @@ static int anx7625_parse_dt(struct device *dev,
if (of_property_read_bool(np, "analogix,audio-enable"))
pdata->audio_en = 1;
+ /*
+* Don't bother finding a panel if a Type-C `mode-switch` property is
+* present in one of the endpoints.
+*/
+ for_each_endpoint_of_node(np, sw) {
+ if (of_property_read_bool(sw, "mode-switch")) {
+ of_node_put(sw);
+ return 0;
+ }
+ }
+
pdata->panel_bridge = devm_drm_of_get_bridge(dev, np, 1, 0);
if (IS_ERR(pdata->panel_bridge)) {
if (PTR_ERR(pdata->panel_bridge) == -ENODEV) {
--
2.38.1.584.g0f3c55d4c2-goog
[PATCH v6 5/7] drm/bridge: anx7625: Register Type C mode switches
Register USB Type-C mode switches when the "mode-switch" property and
relevant port are available in Device Tree. Configure the crosspoint
switch based on the entered alternate mode for a specific Type-C
connector.
Signed-off-by: Pin-yen Lin
---
Changes in v6:
- Squashed to a single patch
drivers/gpu/drm/bridge/analogix/Kconfig | 1 +
drivers/gpu/drm/bridge/analogix/anx7625.c | 169 ++
drivers/gpu/drm/bridge/analogix/anx7625.h | 20 +++
3 files changed, 190 insertions(+)
diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig
b/drivers/gpu/drm/bridge/analogix/Kconfig
index 173dada218ec..992b43ed1dd7 100644
--- a/drivers/gpu/drm/bridge/analogix/Kconfig
+++ b/drivers/gpu/drm/bridge/analogix/Kconfig
@@ -34,6 +34,7 @@ config DRM_ANALOGIX_ANX7625
tristate "Analogix Anx7625 MIPI to DP interface support"
depends on DRM
depends on OF
+ depends on TYPEC || TYPEC=n
select DRM_DISPLAY_DP_HELPER
select DRM_DISPLAY_HDCP_HELPER
select DRM_DISPLAY_HELPER
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index b9e3d6ad8846..2485e61f5cab 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -15,6 +15,8 @@
#include
#include
#include
+#include
+#include
#include
#include
@@ -2573,6 +2575,167 @@ static void anx7625_runtime_disable(void *data)
pm_runtime_disable(data);
}
+static void anx7625_set_crosspoint_switch(struct anx7625_data *ctx,
+ enum typec_orientation orientation)
+{
+ if (orientation == TYPEC_ORIENTATION_NORMAL) {
+ anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_0,
+ SW_SEL1_SSRX_RX1 | SW_SEL1_DPTX0_RX2);
+ anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_1,
+ SW_SEL2_SSTX_TX1 | SW_SEL2_DPTX1_TX2);
+ } else if (orientation == TYPEC_ORIENTATION_REVERSE) {
+ anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_0,
+ SW_SEL1_SSRX_RX2 | SW_SEL1_DPTX0_RX1);
+ anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_1,
+ SW_SEL2_SSTX_TX2 | SW_SEL2_DPTX1_TX1);
+ }
+}
+
+static void anx7625_typec_two_ports_update(struct anx7625_data *ctx)
+{
+ if (ctx->typec_ports[0].dp_connected &&
ctx->typec_ports[1].dp_connected)
+ /* Both ports available, do nothing to retain the current one.
*/
+ return;
+ else if (ctx->typec_ports[0].dp_connected)
+ anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_NORMAL);
+ else if (ctx->typec_ports[1].dp_connected)
+ anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_REVERSE);
+}
+
+static int anx7625_typec_mux_set(struct typec_mux_dev *mux,
+struct typec_mux_state *state)
+{
+ struct anx7625_port_data *data = typec_mux_get_drvdata(mux);
+ struct anx7625_data *ctx = data->ctx;
+ struct device *dev = &ctx->client->dev;
+ bool new_dp_connected, old_dp_connected;
+
+ if (ctx->num_typec_switches == 1)
+ return 0;
+
+ old_dp_connected = ctx->typec_ports[0].dp_connected ||
ctx->typec_ports[1].dp_connected;
+
+ data->dp_connected = (state->alt && state->alt->svid ==
USB_TYPEC_DP_SID &&
+ state->alt->mode == USB_TYPEC_DP_MODE);
+
+ dev_dbg(dev, "mux_set dp_connected: c0=%d, c1=%d\n",
+ ctx->typec_ports[0].dp_connected,
ctx->typec_ports[1].dp_connected);
+
+ new_dp_connected = ctx->typec_ports[0].dp_connected ||
ctx->typec_ports[1].dp_connected;
+
+ /* dp on, power on first */
+ if (!old_dp_connected && new_dp_connected)
+ pm_runtime_get_sync(dev);
+
+ anx7625_typec_two_ports_update(ctx);
+
+ /* dp off, power off last */
+ if (old_dp_connected && !new_dp_connected)
+ pm_runtime_put_sync(dev);
+
+ return 0;
+}
+
+static int anx7625_register_mode_switch(struct device *dev, struct device_node
*node,
+ struct anx7625_data *ctx)
+{
+ struct anx7625_port_data *port_data;
+ struct typec_mux_desc mux_desc = {};
+ char name[32];
+ u32 dp_lanes[2];
+ int ret, num_lanes, i, port_num = -1;
+
+ num_lanes = drm_of_get_data_lanes_count(node, 0, 2);
+ if (num_lanes < 0) {
+ dev_err(dev, "Error on getting data lanes count: %d\n",
num_lanes);
+ return num_lanes;
+ }
+
+ ret = of_property_read_u32_array(node, "data-lanes", dp_lanes,
num_lanes);
+ if (ret) {
+ dev_err(dev, "Failed to read the data-lanes variable: %d\n",
+ ret);
+ return ret;
+ }
+
+ /*
+* <0 1> refers to SSRX1/SS
[PATCH v6 2/7] platform/chrome: cros_ec_typec: Purge blocking switch devlinks
From: Prashant Malani
When using OF graph, the fw_devlink code will create links between the
individual port driver (cros-ec-typec here) and the parent device for
a Type-C switch (like mode-switch). Since the mode-switch will in turn
have the usb-c-connector (i.e the child of the port driver) as a
supplier, fw_devlink will not be able to resolve the cyclic dependency
correctly.
As a result, the mode-switch driver probe() never runs, so mode-switches
are never registered. Because of that, the port driver probe constantly
fails with -EPROBE_DEFER, because the Type-C connector class requires all
switch devices to be registered prior to port registration.
To break this deadlock and allow the mode-switch registration to occur,
purge all the usb-c-connector nodes' absent suppliers. This eliminates
the connector as a supplier for a switch and allows it to be probed.
Signed-off-by: Prashant Malani
Signed-off-by: Pin-yen Lin
---
Changes in v6:
- New in v6
drivers/platform/chrome/cros_ec_typec.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/platform/chrome/cros_ec_typec.c
b/drivers/platform/chrome/cros_ec_typec.c
index 2a7ff14dc37e..f74e01d18ef3 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -382,6 +382,15 @@ static int cros_typec_init_ports(struct cros_typec_data
*typec)
return -EINVAL;
}
+ /*
+* OF graph may have set up some device links with switches, since
connectors have their
+* own compatible. Purge these to avoid a deadlock in switch probe (the
switch mistakenly
+* assumes the connector is a supplier).
+*/
+ if (dev->of_node)
+ device_for_each_child_node(dev, fwnode)
+ fw_devlink_purge_absent_suppliers(fwnode);
+
/* DT uses "reg" to specify port number. */
port_prop = dev->of_node ? "reg" : "port-number";
device_for_each_child_node(dev, fwnode) {
--
2.38.1.584.g0f3c55d4c2-goog
[PATCH v6 3/7] dt-bindings: drm/bridge: anx7625: Add mode-switch support
Analogix 7625 can be used in systems to switch the DP traffic between
two downstreams, which can be USB Type-C DisplayPort alternate mode
lane or regular DisplayPort output ports.
Update the binding to accommodate this usage by introducing a
data-lanes and a mode-switch property on endpoints.
Also include the link to the product brief in the bindings.
Signed-off-by: Pin-yen Lin
---
Changes in v6:
- Remove switches node and use endpoints and data-lanes property to
describe the connections.
.../display/bridge/analogix,anx7625.yaml | 73 ++-
1 file changed, 71 insertions(+), 2 deletions(-)
diff --git
a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
index 4590186c4a0b..5fdbf1f3bab8 100644
--- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
@@ -12,7 +12,8 @@ maintainers:
description: |
The ANX7625 is an ultra-low power 4K Mobile HD Transmitter
- designed for portable devices.
+ designed for portable devices. Product brief is available at
+
https://www.analogix.com/en/system/files/AA-002291-PB-6-ANX7625_ProductBrief.pdf
properties:
compatible:
@@ -112,10 +113,36 @@ properties:
data-lanes: true
port@1:
-$ref: /schemas/graph.yaml#/properties/port
+$ref: /schemas/graph.yaml#/properties/port-base
description:
Video port for panel or connector.
+patternProperties:
+ "^endpoint@[01]$":
+$ref: /schemas/media/video-interfaces.yaml#
+type: object
+unevaluatedProperties: false
+
+properties:
+ reg:
+maxItems: 1
+
+ remote-endpoint: true
+
+ data-lanes:
+minItems: 1
+uniqueItems: true
+items:
+ - enum: [ 0, 1, 2, 3]
+
+ mode-switch:
+type: boolean
+description: Register this node as a Type-C mode switch or not.
+
+required:
+ - reg
+ - remote-endpoint
+
required:
- port@0
- port@1
@@ -186,3 +213,45 @@ examples:
};
};
};
+ - |
+&i2c3 {
+ anx_bridge_dp: anx7625-dp@58 {
+ compatible = "analogix,anx7625";
+ reg = <0x58>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&anx7625_dp_pins>;
+ enable-gpios = <&pio 176 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&pio 177 GPIO_ACTIVE_HIGH>;
+ vdd10-supply = <&pp1100_dpbrdg>;
+ vdd18-supply = <&pp1800_dpbrdg_dx>;
+ vdd33-supply = <&pp3300_dpbrdg_dx>;
+ analogix,audio-enable;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ anx7625_dp_in: endpoint {
+ bus-type = <7>;
+ remote-endpoint = <&dpi_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ anx_typec0: endpoint@0 {
+ mode-switch;
+ data-lanes = <0 1>;
+ remote-endpoint = <&typec_port0>;
+ };
+ anx_typec1: endpoint@1 {
+ mode-switch;
+ data-lanes = <2 3>;
+ remote-endpoint = <&typec_port1>;
+ };
+ };
+ };
+ };
+};
--
2.38.1.584.g0f3c55d4c2-goog
[PATCH v6 1/7] device property: Add remote endpoint to devcon matcher
From: Prashant Malani
When searching the device graph for device matches, check the
remote-endpoint itself for a match.
Some drivers register devices for individual endpoints. This allows
the matcher code to evaluate those for a match too, instead
of only looking at the remote parent devices. This is required when a
device supports two mode switches in its endpoints, so we can't simply
register the mode switch with the parent node.
Signed-off-by: Prashant Malani
Signed-off-by: Pin-yen Lin
---
Changes in v6:
- New in v6
drivers/base/property.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 4d6278a84868..2ab8be8ca45e 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1223,6 +1223,21 @@ static unsigned int fwnode_graph_devcon_matches(struct
fwnode_handle *fwnode,
break;
}
+ /*
+* Some drivers may register devices for endpoints. Check
+* the remote-endpoints for matches in addition to the remote
+* port parent.
+*/
+ node = fwnode_graph_get_remote_endpoint(ep);
+ if (fwnode_device_is_available(node)) {
+ ret = match(node, con_id, data);
+ if (ret) {
+ if (matches)
+ matches[count] = ret;
+ count++;
+ }
+ }
+
node = fwnode_graph_get_remote_port_parent(ep);
if (!fwnode_device_is_available(node)) {
fwnode_handle_put(node);
--
2.38.1.584.g0f3c55d4c2-goog
[PATCH v6 0/7] Register Type-C mode-switch in DP bridge endpoints
This series introduces bindings for anx7625/it6505 to register Type-C mode-switch in their output endpoints, and use data-lanes property to describe the pin connections. The first two patch modifies fwnode_graph_devcon_matches and cros_typec_init_ports to enable the registration of the switches. Patch 3~5 introduce the bindings for anx7625 and the corresponding driver modifications. Patch 6~7 add similar bindings and driver changes for it6505. v5: https://lore.kernel.org/linux-usb/20220622173605.1168416-1-pmal...@chromium.org/ Changes in v6: - Dropped typec-switch binding and use endpoints and data-lanes properties to describe the pin connections - Changed the driver implementation to accommodate with the new bindings - Changed it6505_typec_mux_set callback function to accommodate with the latest drm-misc patches - Added new patches (patch 1,2,4) to fix probing issues - Merged it6505/anx7625 driver changes into a single patch Pin-yen Lin (5): dt-bindings: drm/bridge: anx7625: Add mode-switch support drm/bridge: anx7625: Check for Type-C during panel registration drm/bridge: anx7625: Register Type C mode switches dt/bindings: drm/bridge: it6505: Add mode-switch support drm/bridge: it6505: Register Type C mode switches Prashant Malani (2): device property: Add remote endpoint to devcon matcher platform/chrome: cros_ec_typec: Purge blocking switch devlinks .../display/bridge/analogix,anx7625.yaml | 73 ++- .../bindings/display/bridge/ite,it6505.yaml | 94 +++- drivers/base/property.c | 15 ++ drivers/gpu/drm/bridge/Kconfig| 1 + drivers/gpu/drm/bridge/analogix/Kconfig | 1 + drivers/gpu/drm/bridge/analogix/anx7625.c | 182 +++- drivers/gpu/drm/bridge/analogix/anx7625.h | 20 ++ drivers/gpu/drm/bridge/ite-it6505.c | 205 +- drivers/platform/chrome/cros_ec_typec.c | 9 + 9 files changed, 589 insertions(+), 11 deletions(-) -- 2.38.1.584.g0f3c55d4c2-goog
RE: [PATCH] swsmu/amdgpu_smu: Fix the wrong if-condition
[AMD Official Use Only - General]
Reviewed-by: Evan Quan
> -Original Message-
> From: amd-gfx On Behalf Of Yu
> Songping
> Sent: Thursday, November 24, 2022 9:53 AM
> To: airl...@gmail.com; dan...@ffwll.ch
> Cc: dri-devel@lists.freedesktop.org; amd-...@lists.freedesktop.org; linux-
> ker...@vger.kernel.org
> Subject: [PATCH] swsmu/amdgpu_smu: Fix the wrong if-condition
>
> The logical operator '&&' will make
> smu->ppt_funcs->set_gfx_power_up_by_imu segment fault when
> ppt_funcs is
> smu->NULL.
>
> Signed-off-by: Yu Songping
> ---
> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index b880f4d7d67e..1cb728b0b7ee 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -161,7 +161,7 @@ int smu_get_dpm_freq_range(struct smu_context
> *smu,
>
> int smu_set_gfx_power_up_by_imu(struct smu_context *smu) {
> - if (!smu->ppt_funcs && !smu->ppt_funcs-
> >set_gfx_power_up_by_imu)
> + if (!smu->ppt_funcs || !smu->ppt_funcs-
> >set_gfx_power_up_by_imu)
> return -EOPNOTSUPP;
>
> return smu->ppt_funcs->set_gfx_power_up_by_imu(smu);
> --
> 2.17.1
<>
Re: [PATCH] drm/amdgpu: add mb for si
That's not a patch but some binary file? Christian. Am 24.11.22 um 11:04 schrieb Quan, Evan: [AMD Official Use Only - General] Could the attached patch help? Evan -Original Message- From: amd-gfx On Behalf Of ??? Sent: Friday, November 18, 2022 5:25 PM To: Michel Dänzer ; Koenig, Christian ; Deucher, Alexander Cc: amd-...@lists.freedesktop.org; Pan, Xinhui ; linux-ker...@vger.kernel.org; dri-devel@lists.freedesktop.org Subject: Re: [PATCH] drm/amdgpu: add mb for si 在 2022/11/18 17:18, Michel Dänzer 写道: On 11/18/22 09:01, Christian König wrote: Am 18.11.22 um 08:48 schrieb Zhenneng Li: During reboot test on arm64 platform, it may failure on boot, so add this mb in smc. The error message are as follows: [ 6.996395][ 7] [ T295] [drm:amdgpu_device_ip_late_init [amdgpu]] *ERROR* late_init of IP block failed -22 [ 7.006919][ 7] [ T295] amdgpu :04:00.0: amdgpu_device_ip_late_init failed [ 7.014224][ 7] [ T295] amdgpu :04:00.0: Fatal error during GPU init Memory barries are not supposed to be sprinkled around like this, you need to give a detailed explanation why this is necessary. Regards, Christian. Signed-off-by: Zhenneng Li --- drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c b/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c index 8f994ffa9cd1..c7656f22278d 100644 --- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c @@ -155,6 +155,8 @@ bool amdgpu_si_is_smc_running(struct amdgpu_device *adev) u32 rst = RREG32_SMC(SMC_SYSCON_RESET_CNTL); u32 clk = RREG32_SMC(SMC_SYSCON_CLOCK_CNTL_0); + mb(); + if (!(rst & RST_REG) && !(clk & CK_DISABLE)) return true; In particular, it makes no sense in this specific place, since it cannot directly affect the values of rst & clk. I thinks so too. But when I do reboot test using nine desktop machines, there maybe report this error on one or two machines after Hundreds of times or Thousands of times reboot test, at the beginning, I use msleep() instead of mb(), these two methods are all works, but I don't know what is the root case. I use this method on other verdor's oland card, this error message are reported again. What could be the root reason? test environmen: graphics card: OLAND 0x1002:0x6611 0x1642:0x1869 0x87 driver: amdgpu os: ubuntu 2004 platform: arm64 kernel: 5.4.18
RE: [PATCH] drm/amdgpu: add mb for si
[AMD Official Use Only - General] Could the attached patch help? Evan > -Original Message- > From: amd-gfx On Behalf Of ??? > Sent: Friday, November 18, 2022 5:25 PM > To: Michel Dänzer ; Koenig, Christian > ; Deucher, Alexander > > Cc: amd-...@lists.freedesktop.org; Pan, Xinhui ; > linux-ker...@vger.kernel.org; dri-devel@lists.freedesktop.org > Subject: Re: [PATCH] drm/amdgpu: add mb for si > > > 在 2022/11/18 17:18, Michel Dänzer 写道: > > On 11/18/22 09:01, Christian König wrote: > >> Am 18.11.22 um 08:48 schrieb Zhenneng Li: > >>> During reboot test on arm64 platform, it may failure on boot, so add > >>> this mb in smc. > >>> > >>> The error message are as follows: > >>> [ 6.996395][ 7] [ T295] [drm:amdgpu_device_ip_late_init > >>> [amdgpu]] *ERROR* > >>> late_init of IP block failed -22 [ > >>> 7.006919][ 7] [ T295] amdgpu :04:00.0: > >>> amdgpu_device_ip_late_init failed [ 7.014224][ 7] [ T295] amdgpu > >>> :04:00.0: Fatal error during GPU init > >> Memory barries are not supposed to be sprinkled around like this, you > need to give a detailed explanation why this is necessary. > >> > >> Regards, > >> Christian. > >> > >>> Signed-off-by: Zhenneng Li > >>> --- > >>> drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c | 2 ++ > >>> 1 file changed, 2 insertions(+) > >>> > >>> diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c > >>> b/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c > >>> index 8f994ffa9cd1..c7656f22278d 100644 > >>> --- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c > >>> +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_smc.c > >>> @@ -155,6 +155,8 @@ bool amdgpu_si_is_smc_running(struct > >>> amdgpu_device *adev) > >>> u32 rst = RREG32_SMC(SMC_SYSCON_RESET_CNTL); > >>> u32 clk = RREG32_SMC(SMC_SYSCON_CLOCK_CNTL_0); > >>> + mb(); > >>> + > >>> if (!(rst & RST_REG) && !(clk & CK_DISABLE)) > >>> return true; > > In particular, it makes no sense in this specific place, since it cannot > > directly > affect the values of rst & clk. > > I thinks so too. > > But when I do reboot test using nine desktop machines, there maybe report > this error on one or two machines after Hundreds of times or Thousands of > times reboot test, at the beginning, I use msleep() instead of mb(), these > two methods are all works, but I don't know what is the root case. > > I use this method on other verdor's oland card, this error message are > reported again. > > What could be the root reason? > > test environmen: > > graphics card: OLAND 0x1002:0x6611 0x1642:0x1869 0x87 > > driver: amdgpu > > os: ubuntu 2004 > > platform: arm64 > > kernel: 5.4.18 > > > <>
[PATCH 3/3] drm/amdgpu: generally allow over-commit during BO allocation
We already fallback to a dummy BO with no backing store when we
allocate GDS,GWS and OA resources and to GTT when we allocate VRAM.
Drop all those workarounds and generalize this for GTT as well. This
fixes ENOMEM issues with runaway applications which try to allocate/free
GTT in a loop and are otherwise only limited by the CPU speed.
The CS will wait for the cleanup of freed up BOs to satisfy the
various domain specific limits and so effectively throttle those
buggy applications down to a sane allocation behavior again.
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c| 16 +++-
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 6 +-
2 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 8ef31d687ef3..286509e5e17e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -112,7 +112,7 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev,
unsigned long size,
bp.resv = resv;
bp.preferred_domain = initial_domain;
bp.flags = flags;
- bp.domain = initial_domain;
+ bp.domain = initial_domain | AMDGPU_GEM_DOMAIN_CPU;
bp.bo_ptr_size = sizeof(struct amdgpu_bo);
r = amdgpu_bo_create_user(adev, &bp, &ubo);
@@ -331,20 +331,10 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void
*data,
}
initial_domain = (u32)(0x & args->in.domains);
-retry:
r = amdgpu_gem_object_create(adev, size, args->in.alignment,
-initial_domain,
-flags, ttm_bo_type_device, resv, &gobj);
+initial_domain, flags, ttm_bo_type_device,
+resv, &gobj);
if (r && r != -ERESTARTSYS) {
- if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
- flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
- goto retry;
- }
-
- if (initial_domain == AMDGPU_GEM_DOMAIN_VRAM) {
- initial_domain |= AMDGPU_GEM_DOMAIN_GTT;
- goto retry;
- }
DRM_DEBUG("Failed to allocate GEM object (%llu, %d, %llu,
%d)\n",
size, initial_domain, args->in.alignment, r);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 974e85d8b6cc..919bbea2e3ac 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -581,11 +581,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
bo->tbo.bdev = &adev->mman.bdev;
- if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA |
- AMDGPU_GEM_DOMAIN_GDS))
- amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_CPU);
- else
- amdgpu_bo_placement_from_domain(bo, bp->domain);
+ amdgpu_bo_placement_from_domain(bo, bp->domain);
if (bp->type == ttm_bo_type_kernel)
bo->tbo.priority = 1;
--
2.34.1
[PATCH 2/3] drm/ttm: use per BO cleanup workers
Instead of a single worker going over the list of delete BOs in regular
intervals use a per BO worker which blocks for the resv object and
locking of the BO.
This not only simplifies the handling massively, but also results in
much better response time when cleaning up buffers.
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
drivers/gpu/drm/i915/i915_gem.c| 2 +-
drivers/gpu/drm/i915/intel_region_ttm.c| 2 +-
drivers/gpu/drm/ttm/ttm_bo.c | 112 -
drivers/gpu/drm/ttm/ttm_bo_util.c | 1 -
drivers/gpu/drm/ttm/ttm_device.c | 24 ++---
include/drm/ttm/ttm_bo_api.h | 18 +---
include/drm/ttm/ttm_device.h | 7 +-
8 files changed, 57 insertions(+), 111 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index ff2ae0be2c28..05f458d67eb8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3966,7 +3966,7 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev)
amdgpu_fence_driver_hw_fini(adev);
if (adev->mman.initialized)
- flush_delayed_work(&adev->mman.bdev.wq);
+ drain_workqueue(adev->mman.bdev.wq);
if (adev->pm_sysfs_en)
amdgpu_pm_sysfs_fini(adev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 299f94a9fb87..413c9dbe5be1 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1099,7 +1099,7 @@ void i915_gem_drain_freed_objects(struct drm_i915_private
*i915)
{
while (atomic_read(&i915->mm.free_count)) {
flush_work(&i915->mm.free_work);
- flush_delayed_work(&i915->bdev.wq);
+ drain_workqueue(i915->bdev.wq);
rcu_barrier();
}
}
diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c
b/drivers/gpu/drm/i915/intel_region_ttm.c
index cf89d0c2a2d9..657bbc16a48a 100644
--- a/drivers/gpu/drm/i915/intel_region_ttm.c
+++ b/drivers/gpu/drm/i915/intel_region_ttm.c
@@ -132,7 +132,7 @@ int intel_region_ttm_fini(struct intel_memory_region *mem)
break;
msleep(20);
- flush_delayed_work(&mem->i915->bdev.wq);
+ drain_workqueue(mem->i915->bdev.wq);
}
/* If we leaked objects, Don't free the region causing use after free */
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index b77262a623e0..4749b65bedc4 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -280,14 +280,13 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object
*bo,
ret = 0;
}
- if (ret || unlikely(list_empty(&bo->ddestroy))) {
+ if (ret) {
if (unlock_resv)
dma_resv_unlock(bo->base.resv);
spin_unlock(&bo->bdev->lru_lock);
return ret;
}
- list_del_init(&bo->ddestroy);
spin_unlock(&bo->bdev->lru_lock);
ttm_bo_cleanup_memtype_use(bo);
@@ -300,47 +299,21 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object
*bo,
}
/*
- * Traverse the delayed list, and call ttm_bo_cleanup_refs on all
- * encountered buffers.
+ * Block for the dma_resv object to become idle, lock the buffer and clean up
+ * the resource and tt object.
*/
-bool ttm_bo_delayed_delete(struct ttm_device *bdev, bool remove_all)
+static void ttm_bo_delayed_delete(struct work_struct *work)
{
- struct list_head removed;
- bool empty;
-
- INIT_LIST_HEAD(&removed);
-
- spin_lock(&bdev->lru_lock);
- while (!list_empty(&bdev->ddestroy)) {
- struct ttm_buffer_object *bo;
-
- bo = list_first_entry(&bdev->ddestroy, struct ttm_buffer_object,
- ddestroy);
- list_move_tail(&bo->ddestroy, &removed);
- if (!ttm_bo_get_unless_zero(bo))
- continue;
-
- if (remove_all || bo->base.resv != &bo->base._resv) {
- spin_unlock(&bdev->lru_lock);
- dma_resv_lock(bo->base.resv, NULL);
-
- spin_lock(&bdev->lru_lock);
- ttm_bo_cleanup_refs(bo, false, !remove_all, true);
-
- } else if (dma_resv_trylock(bo->base.resv)) {
- ttm_bo_cleanup_refs(bo, false, !remove_all, true);
- } else {
- spin_unlock(&bdev->lru_lock);
- }
+ struct ttm_buffer_object *bo;
- ttm_bo_put(bo);
- spin_lock(&bdev->lru_lock);
- }
- list_splice_tail(&removed, &bdev->ddestroy);
- empty = list_empty(&bdev->ddestroy);
- spin_unlock(&bdev->lru_lock);
+ bo = container_of(work, typeof(*bo), delayed_delete);
-
[PATCH 1/3] drm/ttm: remove ttm_bo_(un)lock_delayed_workqueue
Those functions never worked correctly since it is still perfectly
possible that a buffer object is released and the background worker
restarted even after calling them.
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 6 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 +---
drivers/gpu/drm/radeon/radeon_device.c | 5 -
drivers/gpu/drm/radeon/radeon_pm.c | 4 +---
drivers/gpu/drm/ttm/ttm_bo.c| 14 --
include/drm/ttm/ttm_bo_api.h| 16
6 files changed, 3 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index de61a85c4b02..8c57a5b7ecba 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -1717,7 +1717,7 @@ static void amdgpu_ib_preempt_mark_partial_job(struct
amdgpu_ring *ring)
static int amdgpu_debugfs_ib_preempt(void *data, u64 val)
{
- int r, resched, length;
+ int r, length;
struct amdgpu_ring *ring;
struct dma_fence **fences = NULL;
struct amdgpu_device *adev = (struct amdgpu_device *)data;
@@ -1747,8 +1747,6 @@ static int amdgpu_debugfs_ib_preempt(void *data, u64 val)
/* stop the scheduler */
kthread_park(ring->sched.thread);
- resched = ttm_bo_lock_delayed_workqueue(&adev->mman.bdev);
-
/* preempt the IB */
r = amdgpu_ring_preempt_ib(ring);
if (r) {
@@ -1785,8 +1783,6 @@ static int amdgpu_debugfs_ib_preempt(void *data, u64 val)
up_read(&adev->reset_domain->sem);
- ttm_bo_unlock_delayed_workqueue(&adev->mman.bdev, resched);
-
pro_end:
kfree(fences);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index cc65df9f2419..ff2ae0be2c28 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3965,10 +3965,8 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev)
}
amdgpu_fence_driver_hw_fini(adev);
- if (adev->mman.initialized) {
+ if (adev->mman.initialized)
flush_delayed_work(&adev->mman.bdev.wq);
- ttm_bo_lock_delayed_workqueue(&adev->mman.bdev);
- }
if (adev->pm_sysfs_en)
amdgpu_pm_sysfs_fini(adev);
diff --git a/drivers/gpu/drm/radeon/radeon_device.c
b/drivers/gpu/drm/radeon/radeon_device.c
index a556b6be1137..ea10306809cf 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1768,7 +1768,6 @@ int radeon_gpu_reset(struct radeon_device *rdev)
bool saved = false;
int i, r;
- int resched;
down_write(&rdev->exclusive_lock);
@@ -1780,8 +1779,6 @@ int radeon_gpu_reset(struct radeon_device *rdev)
atomic_inc(&rdev->gpu_reset_counter);
radeon_save_bios_scratch_regs(rdev);
- /* block TTM */
- resched = ttm_bo_lock_delayed_workqueue(&rdev->mman.bdev);
radeon_suspend(rdev);
radeon_hpd_fini(rdev);
@@ -1840,8 +1837,6 @@ int radeon_gpu_reset(struct radeon_device *rdev)
/* reset hpd state */
radeon_hpd_init(rdev);
- ttm_bo_unlock_delayed_workqueue(&rdev->mman.bdev, resched);
-
rdev->in_reset = true;
rdev->needs_reset = false;
diff --git a/drivers/gpu/drm/radeon/radeon_pm.c
b/drivers/gpu/drm/radeon/radeon_pm.c
index 04c693ca419a..cbc554928bcc 100644
--- a/drivers/gpu/drm/radeon/radeon_pm.c
+++ b/drivers/gpu/drm/radeon/radeon_pm.c
@@ -1853,11 +1853,10 @@ static bool radeon_pm_debug_check_in_vbl(struct
radeon_device *rdev, bool finish
static void radeon_dynpm_idle_work_handler(struct work_struct *work)
{
struct radeon_device *rdev;
- int resched;
+
rdev = container_of(work, struct radeon_device,
pm.dynpm_idle_work.work);
- resched = ttm_bo_lock_delayed_workqueue(&rdev->mman.bdev);
mutex_lock(&rdev->pm.mutex);
if (rdev->pm.dynpm_state == DYNPM_STATE_ACTIVE) {
int not_processed = 0;
@@ -1908,7 +1907,6 @@ static void radeon_dynpm_idle_work_handler(struct
work_struct *work)
msecs_to_jiffies(RADEON_IDLE_LOOP_MS));
}
mutex_unlock(&rdev->pm.mutex);
- ttm_bo_unlock_delayed_workqueue(&rdev->mman.bdev, resched);
}
/*
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index c3f4b33136e5..b77262a623e0 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -418,20 +418,6 @@ void ttm_bo_put(struct ttm_buffer_object *bo)
}
EXPORT_SYMBOL(ttm_bo_put);
-int ttm_bo_lock_delayed_workqueue(struct ttm_device *bdev)
-{
- return cancel_delayed_work_sync(&bdev->wq);
-}
-EXPORT_SYMBOL(ttm_bo_lock_delayed_workqueue);
-
-void ttm_bo_unlock_delayed_workqueue(struct ttm_device *bdev, int resc
Re: [PATCH 1/6] ASoC: hdmi-codec: add PCM trigger operator
On Thu, Nov 24, 2022 at 1:52 PM Allen-KH Cheng (程冠勳) wrote: > We would want to confirm with you as we work on the hdmi-codec for the > mt8186. > > Do you have any plan to continue working on this series? We may take > over as well. No (from testing's perspective). Please take over it as you require.
Re: [PATCH v3 2/2] drm/i915: Never return 0 if not all requests retired
On Wednesday, 23 November 2022 13:57:26 CET Tvrtko Ursulin wrote: > > On 23/11/2022 09:28, Janusz Krzysztofik wrote: > > Hi Tvrtko, > > > > Thanks for your comments. > > > > On Tuesday, 22 November 2022 11:50:38 CET Tvrtko Ursulin wrote: > >> > >> On 21/11/2022 14:56, Janusz Krzysztofik wrote: > >>> Users of intel_gt_retire_requests_timeout() expect 0 return value on > >>> success. However, we have no protection from passing back 0 potentially > >>> returned by a call to dma_fence_wait_timeout() when it succedes right > >>> after its timeout has expired. > >> > >> Is this talking about a potential weakness, or ambiguous kerneldoc, of > >> dma_fence_wait_timeout, dma_fence_default_wait and > >> i915_request_wait_timeout? They appear to say 0 return means timeout, > >> implying unsignaled fence. In other words signaled must return positive > >> remaining timeout. Implementations seems to allow a race which indeed > >> appears that return 0 and signaled fence is possible. > > > > While my initial analysis was indeed focused on inconsistent semantics of 0 > > return values from different dma_fence_default_wait() backends, I should > > have > > also mentioned in this commit description that users may perfectly > > call intel_gt_retire_requests_timeout() with 0 timeout, in which case the > > false positive 0 value can be returned regardless of > > dma_fence_wait_timeout() > > potential issues. Would you like me to reword and resubmit? > > Not sure yet. > > So the only caller which passes in zero to > intel_gt_retire_requests_timeout appears to be intel_gt_retire_requests > and it eats the return value anyway so this patch is immaterial for that > one. Right. > I guess it can change how intel_gt_wait_for_idle behaves with short-ish > timeouts. In case this race is hit. But then wouldn't it make sense to > follow up with a patch which addresses this race by re-checking the "is > signaled" when timeout expires, But inside intel_gt_retire_requests_timeout() we generally don't care if fences have been signaled. As long as user requested timeout hasn't expired, we use dma_fence_wait_timeout() as an aid, otherwise we keep trying to retire requests without waiting on fences. If the retirement succeeds then we return 0 (success) regardless of what the return value from the last called dma_fence_wait_timeout() was. If it was 0 then the only useful information is that no more time has been left, and no matter if that 0 meant signaled or not signaled, we must return an error if there are still some requests not retired, I believe. > either in dma_fence_wait_timeout, or to > intel_gt_retire_requests_timeout. Or if not that at least better > document the dma_fence_wait_timeout and/or > intel_gt_retire_requests_timeout. Makes sense? Documenting -- yes, as soon as we get into an agreement on what's the core of this issue -- whether that potential weakness, or ambiguous kerneldoc, of dma_fence_wait_timeout, dma_fence_default_wait and i915_request_wait_timeout, as you've stated, that we have to address somehow, or potentially incorrect direct use of the timeout variable, intended for storing time left to spend on fence waits, as our return value when timeout has expired. And if the former then maybe we should also try to finally resolve that over a year old conflict (whether 0 means signaled on not signaled) inside our implementation of dma_fence_ops.wait, and simply use a wrapper around it for either our internal use if we decide to follow the reference implementation, or for dma_fence_ops use otherwise. Or maybe the reference implementation should be fixed if problematic. I don't feel competent enough to decide. Thanks, Janusz > > Regards, > > Tvrtko > > > > >> If dma_fence_wait can indeed return 0 even when a request is signaled, > >> then how is timeout ?: -ETIME below correct? It isn't a chance for false > >> negative in its' callers? > > > > The goal of intel_gt_retire_requests_timeout() is to retire requests. When > > that goal has been reached, i.e., all requests have been retired, active > > count > > is 0, and 0 is correctly returned, regardless of timeout value. > > > > The value of timeout is used only when there are still pending requests, > > which > > means that the goal hasn't been reached and the function hasn't succeeded. > > Then, no false negative is possible, unlike the false positive that we now > > have when we return 0 while some requests are still pending. > > > > Thanks, > > Janusz > > > >> > >> Regards, > >> > >> Tvrtko > >> > >>> Replace 0 with -ETIME before potentially using the timeout value as return > >>> code, so -ETIME is returned if there are still some requests not retired > >>> after timeout, 0 otherwise. > >>> > >>> v3: Use conditional expression, more compact but also better reflecting > >>> intention standing behind the change. > >>> > >>> v2: Move the added lines down so flush_submission() is not affected. > >>> > >>> Fi
Re: [PATCH] drm/fourcc: Document open source user waiver
On Wed, 23 Nov 2022 at 19:24, Daniel Vetter wrote: > It's a bit a FAQ, and we really can't claim to be the authoritative > source for allocating these numbers used in many standard extensions > if we tell closed source or vendor stacks in general to go away. > > Iirc this was already clarified in some vulkan discussions, but I > can't find that anywhere anymore. At least not in a public link. I seem to recall the policy being set in an IRC discussion at some point (or perhaps during the AFBC merge?). This is a good clarification of what we already do in practice. Acked-by: Daniel Stone
Re: [Linaro-mm-sig] Re: [PATCH] dma-buf: A collection of typo and documentation fixes
Am 24.11.22 um 10:05 schrieb Daniel Vetter:
On Thu, Nov 24, 2022 at 08:03:09AM +0100, Christian König wrote:
Am 23.11.22 um 20:35 schrieb T.J. Mercier:
I've been collecting these typo fixes for a while and it feels like
time to send them in.
Signed-off-by: T.J. Mercier
Acked-by: Christian König
Will you also push this? I think tj doesn't have commit rights yet, and I
somehow can't see the patch locally (I guess it's stuck in moderation).
I was just about to complain that this doesn't apply cleanly to
drm-misc-next.
Trivial problem, one of the typos was just removed by Dimitry a few
weeks ago.
I've fixed that up locally and pushed the result, but nevertheless
please make sure that DMA-buf patches are based on the drm branches.
Thanks,
Christian.
-Daniel
---
drivers/dma-buf/dma-buf.c | 14 +++---
include/linux/dma-buf.h | 6 +++---
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index dd0f83ee505b..614ccd208af4 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -1141,7 +1141,7 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_unmap_attachment, DMA_BUF);
*
* @dmabuf: [in]buffer which is moving
*
- * Informs all attachmenst that they need to destroy and recreated all their
+ * Informs all attachments that they need to destroy and recreate all their
* mappings.
*/
void dma_buf_move_notify(struct dma_buf *dmabuf)
@@ -1159,11 +1159,11 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_move_notify, DMA_BUF);
/**
* DOC: cpu access
*
- * There are mutliple reasons for supporting CPU access to a dma buffer object:
+ * There are multiple reasons for supporting CPU access to a dma buffer object:
*
* - Fallback operations in the kernel, for example when a device is
connected
* over USB and the kernel needs to shuffle the data around first before
- * sending it away. Cache coherency is handled by braketing any transactions
+ * sending it away. Cache coherency is handled by bracketing any transactions
* with calls to dma_buf_begin_cpu_access() and dma_buf_end_cpu_access()
* access.
*
@@ -1190,7 +1190,7 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_move_notify, DMA_BUF);
* replace ION buffers mmap support was needed.
*
* There is no special interfaces, userspace simply calls mmap on the
dma-buf
- * fd. But like for CPU access there's a need to braket the actual access,
+ * fd. But like for CPU access there's a need to bracket the actual access,
* which is handled by the ioctl (DMA_BUF_IOCTL_SYNC). Note that
* DMA_BUF_IOCTL_SYNC can fail with -EAGAIN or -EINTR, in which case it
must
* be restarted.
@@ -1264,10 +1264,10 @@ static int __dma_buf_begin_cpu_access(struct dma_buf
*dmabuf,
* preparations. Coherency is only guaranteed in the specified range for the
* specified access direction.
* @dmabuf: [in]buffer to prepare cpu access for.
- * @direction: [in]length of range for cpu access.
+ * @direction: [in]direction of access.
*
* After the cpu access is complete the caller should call
- * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
+ * dma_buf_end_cpu_access(). Only when cpu access is bracketed by both calls is
* it guaranteed to be coherent with other DMA access.
*
* This function will also wait for any DMA transactions tracked through
@@ -1307,7 +1307,7 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_begin_cpu_access, DMA_BUF);
* actions. Coherency is only guaranteed in the specified range for the
* specified access direction.
* @dmabuf: [in]buffer to complete cpu access for.
- * @direction: [in]length of range for cpu access.
+ * @direction: [in]direction of access.
*
* This terminates CPU access started with dma_buf_begin_cpu_access().
*
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 71731796c8c3..1d61a4f6db35 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -330,7 +330,7 @@ struct dma_buf {
* @lock:
*
* Used internally to serialize list manipulation, attach/detach and
-* vmap/unmap. Note that in many cases this is superseeded by
+* vmap/unmap. Note that in many cases this is superseded by
* dma_resv_lock() on @resv.
*/
struct mutex lock;
@@ -365,7 +365,7 @@ struct dma_buf {
*/
const char *name;
- /** @name_lock: Spinlock to protect name acces for read access. */
+ /** @name_lock: Spinlock to protect name access for read access. */
spinlock_t name_lock;
/**
@@ -402,7 +402,7 @@ struct dma_buf {
* anything the userspace API considers write access.
*
* - Drivers may just always add a write fence, since that only
-* causes unecessarily synchronization, but no correctness issues.
+* causes unnecessary synchronization, but no corre
Re: [PATCH v28 05/11] soc: mediatek: refine code to use mtk_mmsys_update_bits API
Re: [PATCH resend v2] drm/fourcc: Add missing big-endian XRGB1555 and RGB565 formats
Hi Thomas,
On Thu, Nov 24, 2022 at 10:20 AM Thomas Zimmermann wrote:
> Am 24.11.22 um 10:04 schrieb Daniel Vetter:
> > On Thu, Nov 24, 2022 at 09:55:18AM +0100, Geert Uytterhoeven wrote:
> >> Hi Thomas,
> >>
> >> On Thu, Nov 24, 2022 at 9:47 AM Thomas Zimmermann
> >> wrote:
> >>> Am 23.11.22 um 17:43 schrieb Geert Uytterhoeven:
> As of commit eae06120f1974e1a ("drm: refuse ADDFB2 ioctl for broken
> bigendian drivers"), drivers must set the
> quirk_addfb_prefer_host_byte_order quirk to make the drm_mode_addfb()
> compat code work correctly on big-endian machines.
>
> While that works fine for big-endian XRGB and ARGB, which are
> mapped to the existing little-endian BGRX and BGRA formats, it
> does not work for big-endian XRGB1555 and RGB565, as the latter are not
> listed in the format database.
>
> Fix this by adding the missing formats. Limit this to big-endian
> platforms, as there is currently no need to support these formats on
> little-endian platforms.
>
> Fixes: 6960e6da9cec3f66 ("drm: fix drm_mode_addfb() on big endian
> machines.")
> Signed-off-by: Geert Uytterhoeven
> ---
> v2:
> - Use "DRM_FORMAT_foo | DRM_FORMAT_BIG_ENDIAN" instead of
> "DRM_FORMAT_HOST_foo",
> - Turn into a lone patch, as all other patches from series
> https://lore.kernel.org/r/cover.1657300532.git.ge...@linux-m68k.org
> were applied to drm-misc/for-linux-next.
> ---
> drivers/gpu/drm/drm_fourcc.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index e09331bb3bc73f21..265671a7f9134c1f 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -190,6 +190,10 @@ const struct drm_format_info *__drm_format_info(u32
> format)
> { .format = DRM_FORMAT_BGRA5551,.depth = 15,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha =
> true },
> { .format = DRM_FORMAT_RGB565, .depth = 16,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> { .format = DRM_FORMAT_BGR565, .depth = 16,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> +#ifdef __BIG_ENDIAN
> + { .format = DRM_FORMAT_XRGB1555 | DRM_FORMAT_BIG_ENDIAN,
> .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN,
> .depth = 16, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> >>>
> >>> Getting back to the discussion on endianess, I don't understand why the
> >>> BIG_ENDIAN flag is set here. AFAIK these formats are always little
> >>> endian. And the BE flag is set by drivers/userspace if a framebuffer
> >>> has a BE ordering.
> >>>
> >>> It would be better to filter the BE flag in __drm_format_info() before
> >>> the function does the lookup.
> >>
> >> I mentioned that alternative in [2], but rejected it because of the
> >> disadvantages:
> >>- {,__}drm_format_info() returns a pointer to a const object,
> >> whose .format field won't have the DRM_FORMAT_BIG_ENDIAN flag set,
> >> complicating callers,
> >>- All callers need to be updated,
> >>- It is difficult to know which big-endian formats are really
> >> supported, especially as only a few are needed.
> >
> > fwiw this last point is why I think this is the right approach. Long term
> > we might want to add _BE variants of these #defines so that they can be
> > used everywhere and are easy to grep. As long as it's just a handful of
> > places then the very verboy | DRM_FORMAT_BIG_ENDIAN is ok too.
>
> Doesn't that contradict the comment at [1] to some extend? 'DRM formats
> are little endian.' and extra defines are only made for simplifying drivers.
>
> [1]
> https://elixir.bootlin.com/linux/latest/source/include/drm/drm_fourcc.h#L33
>
> >
> > With this approach we can make it _very_ explicit what big endian formats
> > are supported by a driver or other piece in the stack (like fbdev
> > emulation), and I think explicit is what we want with be because it's
> > become such an exception. Otherwise we'll just end up with more terrible
> > cruft like the host endian hacks in the addfb compat code.
>
> To give a different perspective, with format-conversion helpers the
> destination buffer is usually a hardware buffer that can have big-endian
> ordering. So we sometimes have to swap byteorder to make output colors
> look correct. That is the easiest if all formats are in LE and the
> BIG_ENDIAN flag tells us when the swap. With the current multitude of
> formats and B_E flags that can describe the same result, it's all just
> more complicated.
I'm happy to _not_ export the big-endian RGB565
Re: [Intel-gfx] [PATCH v2 2/4] drm/i915: Introduce guard pages to i915_vma
> > > @@ -768,6 +768,9 @@ i915_vma_insert(struct i915_vma *vma, struct > > > i915_gem_ww_ctx *ww, > > > GEM_BUG_ON(!IS_ALIGNED(alignment, I915_GTT_MIN_ALIGNMENT)); > > > GEM_BUG_ON(!is_power_of_2(alignment)); > > > + guard = vma->guard; /* retain guard across rebinds */ > > > + guard = ALIGN(guard, alignment); > > > > Why does guard area needs the same alignment as the requested mapping? What > > about the fact on 32-bit builds guard is 32-bit and alignment u64? > > I guess this just to round up/down guard to something, not > necessarily to that alignment. > > Shall I remove it? or we could just add a comment to explain that this is just to do some rounding in order to avoid weird values of guard. Andi
Re: [PATCH resend v2] drm/fourcc: Add missing big-endian XRGB1555 and RGB565 formats
Hi
Am 24.11.22 um 10:04 schrieb Daniel Vetter:
On Thu, Nov 24, 2022 at 09:55:18AM +0100, Geert Uytterhoeven wrote:
Hi Thomas,
On Thu, Nov 24, 2022 at 9:47 AM Thomas Zimmermann wrote:
Am 23.11.22 um 17:43 schrieb Geert Uytterhoeven:
As of commit eae06120f1974e1a ("drm: refuse ADDFB2 ioctl for broken
bigendian drivers"), drivers must set the
quirk_addfb_prefer_host_byte_order quirk to make the drm_mode_addfb()
compat code work correctly on big-endian machines.
While that works fine for big-endian XRGB and ARGB, which are
mapped to the existing little-endian BGRX and BGRA formats, it
does not work for big-endian XRGB1555 and RGB565, as the latter are not
listed in the format database.
Fix this by adding the missing formats. Limit this to big-endian
platforms, as there is currently no need to support these formats on
little-endian platforms.
Fixes: 6960e6da9cec3f66 ("drm: fix drm_mode_addfb() on big endian machines.")
Signed-off-by: Geert Uytterhoeven
---
v2:
- Use "DRM_FORMAT_foo | DRM_FORMAT_BIG_ENDIAN" instead of
"DRM_FORMAT_HOST_foo",
- Turn into a lone patch, as all other patches from series
https://lore.kernel.org/r/cover.1657300532.git.ge...@linux-m68k.org
were applied to drm-misc/for-linux-next.
---
drivers/gpu/drm/drm_fourcc.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index e09331bb3bc73f21..265671a7f9134c1f 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -190,6 +190,10 @@ const struct drm_format_info *__drm_format_info(u32 format)
{ .format = DRM_FORMAT_BGRA5551,.depth = 15, .num_planes
= 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
{ .format = DRM_FORMAT_RGB565, .depth = 16, .num_planes
= 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
{ .format = DRM_FORMAT_BGR565, .depth = 16, .num_planes
= 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+#ifdef __BIG_ENDIAN
+ { .format = DRM_FORMAT_XRGB1555 | DRM_FORMAT_BIG_ENDIAN, .depth =
15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN, .depth =
16, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
Getting back to the discussion on endianess, I don't understand why the
BIG_ENDIAN flag is set here. AFAIK these formats are always little
endian. And the BE flag is set by drivers/userspace if a framebuffer
has a BE ordering.
It would be better to filter the BE flag in __drm_format_info() before
the function does the lookup.
I mentioned that alternative in [2], but rejected it because of the
disadvantages:
- {,__}drm_format_info() returns a pointer to a const object,
whose .format field won't have the DRM_FORMAT_BIG_ENDIAN flag set,
complicating callers,
- All callers need to be updated,
- It is difficult to know which big-endian formats are really
supported, especially as only a few are needed.
fwiw this last point is why I think this is the right approach. Long term
we might want to add _BE variants of these #defines so that they can be
used everywhere and are easy to grep. As long as it's just a handful of
places then the very verboy | DRM_FORMAT_BIG_ENDIAN is ok too.
Doesn't that contradict the comment at [1] to some extend? 'DRM formats
are little endian.' and extra defines are only made for simplifying drivers.
[1]
https://elixir.bootlin.com/linux/latest/source/include/drm/drm_fourcc.h#L33
With this approach we can make it _very_ explicit what big endian formats
are supported by a driver or other piece in the stack (like fbdev
emulation), and I think explicit is what we want with be because it's
become such an exception. Otherwise we'll just end up with more terrible
cruft like the host endian hacks in the addfb compat code.
To give a different perspective, with format-conversion helpers the
destination buffer is usually a hardware buffer that can have big-endian
ordering. So we sometimes have to swap byteorder to make output colors
look correct. That is the easiest if all formats are in LE and the
BIG_ENDIAN flag tells us when the swap. With the current multitude of
formats and B_E flags that can describe the same result, it's all just
more complicated.
Best regards
Thomas
-Daniel
[2] [PATCH 1/3] drm/fourcc: Add missing big-endian XRGB1555 and RGB565 formats
https://lore.kernel.org/all/0744671ac096a12f0d538906bd324efa71b11400.1657300532.git.ge...@linux-m68k.org
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
--
Thomas Zimmermann
Graphics Driver D
Re: [PATCH] dma-buf: A collection of typo and documentation fixes
On Thu, Nov 24, 2022 at 08:03:09AM +0100, Christian König wrote:
> Am 23.11.22 um 20:35 schrieb T.J. Mercier:
> > I've been collecting these typo fixes for a while and it feels like
> > time to send them in.
> >
> > Signed-off-by: T.J. Mercier
>
> Acked-by: Christian König
Will you also push this? I think tj doesn't have commit rights yet, and I
somehow can't see the patch locally (I guess it's stuck in moderation).
-Daniel
>
> > ---
> > drivers/dma-buf/dma-buf.c | 14 +++---
> > include/linux/dma-buf.h | 6 +++---
> > 2 files changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index dd0f83ee505b..614ccd208af4 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -1141,7 +1141,7 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_unmap_attachment,
> > DMA_BUF);
> >*
> >* @dmabuf: [in]buffer which is moving
> >*
> > - * Informs all attachmenst that they need to destroy and recreated all
> > their
> > + * Informs all attachments that they need to destroy and recreate all their
> >* mappings.
> >*/
> > void dma_buf_move_notify(struct dma_buf *dmabuf)
> > @@ -1159,11 +1159,11 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_move_notify, DMA_BUF);
> > /**
> >* DOC: cpu access
> >*
> > - * There are mutliple reasons for supporting CPU access to a dma buffer
> > object:
> > + * There are multiple reasons for supporting CPU access to a dma buffer
> > object:
> >*
> >* - Fallback operations in the kernel, for example when a device is
> > connected
> >* over USB and the kernel needs to shuffle the data around first before
> > - * sending it away. Cache coherency is handled by braketing any
> > transactions
> > + * sending it away. Cache coherency is handled by bracketing any
> > transactions
> >* with calls to dma_buf_begin_cpu_access() and dma_buf_end_cpu_access()
> >* access.
> >*
> > @@ -1190,7 +1190,7 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_move_notify, DMA_BUF);
> >* replace ION buffers mmap support was needed.
> >*
> >* There is no special interfaces, userspace simply calls mmap on the
> > dma-buf
> > - * fd. But like for CPU access there's a need to braket the actual
> > access,
> > + * fd. But like for CPU access there's a need to bracket the actual
> > access,
> >* which is handled by the ioctl (DMA_BUF_IOCTL_SYNC). Note that
> >* DMA_BUF_IOCTL_SYNC can fail with -EAGAIN or -EINTR, in which case it
> > must
> >* be restarted.
> > @@ -1264,10 +1264,10 @@ static int __dma_buf_begin_cpu_access(struct
> > dma_buf *dmabuf,
> >* preparations. Coherency is only guaranteed in the specified range for
> > the
> >* specified access direction.
> >* @dmabuf: [in]buffer to prepare cpu access for.
> > - * @direction: [in]length of range for cpu access.
> > + * @direction: [in]direction of access.
> >*
> >* After the cpu access is complete the caller should call
> > - * dma_buf_end_cpu_access(). Only when cpu access is braketed by both
> > calls is
> > + * dma_buf_end_cpu_access(). Only when cpu access is bracketed by both
> > calls is
> >* it guaranteed to be coherent with other DMA access.
> >*
> >* This function will also wait for any DMA transactions tracked through
> > @@ -1307,7 +1307,7 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_begin_cpu_access,
> > DMA_BUF);
> >* actions. Coherency is only guaranteed in the specified range for the
> >* specified access direction.
> >* @dmabuf: [in]buffer to complete cpu access for.
> > - * @direction: [in]length of range for cpu access.
> > + * @direction: [in]direction of access.
> >*
> >* This terminates CPU access started with dma_buf_begin_cpu_access().
> >*
> > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > index 71731796c8c3..1d61a4f6db35 100644
> > --- a/include/linux/dma-buf.h
> > +++ b/include/linux/dma-buf.h
> > @@ -330,7 +330,7 @@ struct dma_buf {
> > * @lock:
> > *
> > * Used internally to serialize list manipulation, attach/detach and
> > -* vmap/unmap. Note that in many cases this is superseeded by
> > +* vmap/unmap. Note that in many cases this is superseded by
> > * dma_resv_lock() on @resv.
> > */
> > struct mutex lock;
> > @@ -365,7 +365,7 @@ struct dma_buf {
> > */
> > const char *name;
> > - /** @name_lock: Spinlock to protect name acces for read access. */
> > + /** @name_lock: Spinlock to protect name access for read access. */
> > spinlock_t name_lock;
> > /**
> > @@ -402,7 +402,7 @@ struct dma_buf {
> > * anything the userspace API considers write access.
> > *
> > * - Drivers may just always add a write fence, since that only
> > -* causes unecessarily synchronization, but no correctness issues.
> > +* causes unnecessary synchronization, but no correc
Re: [PATCH 6/6] drm/gud: Use the shadow plane helper
Hi
Am 23.11.22 um 13:37 schrieb Noralf Trønnes:
Den 23.11.2022 11.38, skrev Thomas Zimmermann:
Hi
Am 22.11.22 um 21:58 schrieb Noralf Trønnes via B4 Submission Endpoint:
From: Noralf Trønnes
Use the shadow plane helper to take care of preparing the framebuffer for
CPU access. The synchronous flushing is now done inline without the
use of
a worker. The async path now uses a shadow buffer to hold framebuffer
changes and it doesn't read the framebuffer behind userspace's back
anymore.
Signed-off-by: Noralf Trønnes
---
drivers/gpu/drm/gud/gud_drv.c | 1 +
drivers/gpu/drm/gud/gud_internal.h | 1 +
drivers/gpu/drm/gud/gud_pipe.c | 69
--
3 files changed, 46 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/gud/gud_drv.c
b/drivers/gpu/drm/gud/gud_drv.c
index d57dab104358..5aac7cda0505 100644
--- a/drivers/gpu/drm/gud/gud_drv.c
+++ b/drivers/gpu/drm/gud/gud_drv.c
@@ -365,6 +365,7 @@ static void gud_debugfs_init(struct drm_minor *minor)
static const struct drm_simple_display_pipe_funcs gud_pipe_funcs = {
.check = gud_pipe_check,
.update = gud_pipe_update,
+ DRM_GEM_SIMPLE_DISPLAY_PIPE_SHADOW_PLANE_FUNCS
};
static const struct drm_mode_config_funcs gud_mode_config_funcs = {
diff --git a/drivers/gpu/drm/gud/gud_internal.h
b/drivers/gpu/drm/gud/gud_internal.h
index e351a1f1420d..0d148a6f27aa 100644
--- a/drivers/gpu/drm/gud/gud_internal.h
+++ b/drivers/gpu/drm/gud/gud_internal.h
@@ -43,6 +43,7 @@ struct gud_device {
struct drm_framebuffer *fb;
struct drm_rect damage;
bool prev_flush_failed;
+ void *shadow_buf;
};
static inline struct gud_device *to_gud_device(struct drm_device
*drm)
diff --git a/drivers/gpu/drm/gud/gud_pipe.c
b/drivers/gpu/drm/gud/gud_pipe.c
index dfada6eedc58..7686325f7ee7 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -358,10 +358,10 @@ static void gud_flush_damage(struct gud_device
*gdrm, struct drm_framebuffer *fb
void gud_flush_work(struct work_struct *work)
{
struct gud_device *gdrm = container_of(work, struct gud_device,
work);
- struct iosys_map gem_map = { }, fb_map = { };
+ struct iosys_map shadow_map;
struct drm_framebuffer *fb;
struct drm_rect damage;
- int idx, ret;
+ int idx;
if (!drm_dev_enter(&gdrm->drm, &idx))
return;
@@ -369,6 +369,7 @@ void gud_flush_work(struct work_struct *work)
mutex_lock(&gdrm->damage_lock);
fb = gdrm->fb;
gdrm->fb = NULL;
+ iosys_map_set_vaddr(&shadow_map, gdrm->shadow_buf);
damage = gdrm->damage;
gud_clear_damage(gdrm);
mutex_unlock(&gdrm->damage_lock);
@@ -376,33 +377,33 @@ void gud_flush_work(struct work_struct *work)
if (!fb)
goto out;
- ret = drm_gem_fb_vmap(fb, &gem_map, &fb_map);
- if (ret)
- goto fb_put;
+ gud_flush_damage(gdrm, fb, &shadow_map, true, &damage);
- ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
- if (ret)
- goto vunmap;
-
- /* Imported buffers are assumed to be WriteCombined with uncached
reads */
- gud_flush_damage(gdrm, fb, &fb_map, !fb->obj[0]->import_attach,
&damage);
-
- drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
-vunmap:
- drm_gem_fb_vunmap(fb, &gem_map);
-fb_put:
drm_framebuffer_put(fb);
out:
drm_dev_exit(idx);
}
-static void gud_fb_queue_damage(struct gud_device *gdrm, struct
drm_framebuffer *fb,
- struct drm_rect *damage)
+static int gud_fb_queue_damage(struct gud_device *gdrm, struct
drm_framebuffer *fb,
+ const struct iosys_map *map, struct drm_rect *damage)
{
struct drm_framebuffer *old_fb = NULL;
+ struct iosys_map shadow_map;
mutex_lock(&gdrm->damage_lock);
+ if (!gdrm->shadow_buf) {
+ gdrm->shadow_buf = vzalloc(fb->pitches[0] * fb->height);
+ if (!gdrm->shadow_buf) {
+ mutex_unlock(&gdrm->damage_lock);
+ return -ENOMEM;
+ }
+ }
+
+ iosys_map_set_vaddr(&shadow_map, gdrm->shadow_buf);
+ iosys_map_incr(&shadow_map, drm_fb_clip_offset(fb->pitches[0],
fb->format, damage));
+ drm_fb_memcpy(&shadow_map, fb->pitches, map, fb, damage);
+
It's all good (sans the begin_cpu_access issue) in terms of code, but
the memcpy here can have quite an impact on performace.
Yes if the gud display is the only one in the rendering loop it's not
good. This is on an old HP laptop running the async path (RGB565 over USB):
$ modetest -M gud -s 36:1920x1080 -v
setting mode 1920x1080-60.00Hz on connectors 36, crtc 33
freq: 414.56Hz
freq: 439.71Hz
This is the normal path:
$ modetest -M gud -s 36:1920x1080 -v
setting mode 1920x1080-60.00Hz on connectors 36, crtc 33
freq: 18.11Hz
freq: 17.98Hz
So why not just nuke the async code entirely? It's a userspace problem
and we don't have many(/any?) other drivers with such a workaround. Udl,
our other u
Re: [PATCH resend v2] drm/fourcc: Add missing big-endian XRGB1555 and RGB565 formats
On Thu, Nov 24, 2022 at 09:55:18AM +0100, Geert Uytterhoeven wrote:
> Hi Thomas,
>
> On Thu, Nov 24, 2022 at 9:47 AM Thomas Zimmermann wrote:
> > Am 23.11.22 um 17:43 schrieb Geert Uytterhoeven:
> > > As of commit eae06120f1974e1a ("drm: refuse ADDFB2 ioctl for broken
> > > bigendian drivers"), drivers must set the
> > > quirk_addfb_prefer_host_byte_order quirk to make the drm_mode_addfb()
> > > compat code work correctly on big-endian machines.
> > >
> > > While that works fine for big-endian XRGB and ARGB, which are
> > > mapped to the existing little-endian BGRX and BGRA formats, it
> > > does not work for big-endian XRGB1555 and RGB565, as the latter are not
> > > listed in the format database.
> > >
> > > Fix this by adding the missing formats. Limit this to big-endian
> > > platforms, as there is currently no need to support these formats on
> > > little-endian platforms.
> > >
> > > Fixes: 6960e6da9cec3f66 ("drm: fix drm_mode_addfb() on big endian
> > > machines.")
> > > Signed-off-by: Geert Uytterhoeven
> > > ---
> > > v2:
> > >- Use "DRM_FORMAT_foo | DRM_FORMAT_BIG_ENDIAN" instead of
> > > "DRM_FORMAT_HOST_foo",
> > >- Turn into a lone patch, as all other patches from series
> > > https://lore.kernel.org/r/cover.1657300532.git.ge...@linux-m68k.org
> > > were applied to drm-misc/for-linux-next.
> > > ---
> > > drivers/gpu/drm/drm_fourcc.c | 4
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> > > index e09331bb3bc73f21..265671a7f9134c1f 100644
> > > --- a/drivers/gpu/drm/drm_fourcc.c
> > > +++ b/drivers/gpu/drm/drm_fourcc.c
> > > @@ -190,6 +190,10 @@ const struct drm_format_info *__drm_format_info(u32
> > > format)
> > > { .format = DRM_FORMAT_BGRA5551,.depth = 15,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha =
> > > true },
> > > { .format = DRM_FORMAT_RGB565, .depth = 16,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > > { .format = DRM_FORMAT_BGR565, .depth = 16,
> > > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > > +#ifdef __BIG_ENDIAN
> > > + { .format = DRM_FORMAT_XRGB1555 | DRM_FORMAT_BIG_ENDIAN,
> > > .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > > + { .format = DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN,
> > > .depth = 16, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> >
> > Getting back to the discussion on endianess, I don't understand why the
> > BIG_ENDIAN flag is set here. AFAIK these formats are always little
> > endian. And the BE flag is set by drivers/userspace if a framebuffer
> > has a BE ordering.
> >
> > It would be better to filter the BE flag in __drm_format_info() before
> > the function does the lookup.
>
> I mentioned that alternative in [2], but rejected it because of the
> disadvantages:
> - {,__}drm_format_info() returns a pointer to a const object,
> whose .format field won't have the DRM_FORMAT_BIG_ENDIAN flag set,
> complicating callers,
> - All callers need to be updated,
> - It is difficult to know which big-endian formats are really
> supported, especially as only a few are needed.
fwiw this last point is why I think this is the right approach. Long term
we might want to add _BE variants of these #defines so that they can be
used everywhere and are easy to grep. As long as it's just a handful of
places then the very verboy | DRM_FORMAT_BIG_ENDIAN is ok too.
With this approach we can make it _very_ explicit what big endian formats
are supported by a driver or other piece in the stack (like fbdev
emulation), and I think explicit is what we want with be because it's
become such an exception. Otherwise we'll just end up with more terrible
cruft like the host endian hacks in the addfb compat code.
-Daniel
>
> [2] [PATCH 1/3] drm/fourcc: Add missing big-endian XRGB1555 and RGB565 formats
> https://lore.kernel.org/all/0744671ac096a12f0d538906bd324efa71b11400.1657300532.git.ge...@linux-m68k.org
>
> 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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH resend v2] drm/fourcc: Add missing big-endian XRGB1555 and RGB565 formats
Hi Thomas,
On Thu, Nov 24, 2022 at 9:47 AM Thomas Zimmermann wrote:
> Am 23.11.22 um 17:43 schrieb Geert Uytterhoeven:
> > As of commit eae06120f1974e1a ("drm: refuse ADDFB2 ioctl for broken
> > bigendian drivers"), drivers must set the
> > quirk_addfb_prefer_host_byte_order quirk to make the drm_mode_addfb()
> > compat code work correctly on big-endian machines.
> >
> > While that works fine for big-endian XRGB and ARGB, which are
> > mapped to the existing little-endian BGRX and BGRA formats, it
> > does not work for big-endian XRGB1555 and RGB565, as the latter are not
> > listed in the format database.
> >
> > Fix this by adding the missing formats. Limit this to big-endian
> > platforms, as there is currently no need to support these formats on
> > little-endian platforms.
> >
> > Fixes: 6960e6da9cec3f66 ("drm: fix drm_mode_addfb() on big endian
> > machines.")
> > Signed-off-by: Geert Uytterhoeven
> > ---
> > v2:
> >- Use "DRM_FORMAT_foo | DRM_FORMAT_BIG_ENDIAN" instead of
> > "DRM_FORMAT_HOST_foo",
> >- Turn into a lone patch, as all other patches from series
> > https://lore.kernel.org/r/cover.1657300532.git.ge...@linux-m68k.org
> > were applied to drm-misc/for-linux-next.
> > ---
> > drivers/gpu/drm/drm_fourcc.c | 4
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> > index e09331bb3bc73f21..265671a7f9134c1f 100644
> > --- a/drivers/gpu/drm/drm_fourcc.c
> > +++ b/drivers/gpu/drm/drm_fourcc.c
> > @@ -190,6 +190,10 @@ const struct drm_format_info *__drm_format_info(u32
> > format)
> > { .format = DRM_FORMAT_BGRA5551,.depth = 15,
> > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha =
> > true },
> > { .format = DRM_FORMAT_RGB565, .depth = 16,
> > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > { .format = DRM_FORMAT_BGR565, .depth = 16,
> > .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > +#ifdef __BIG_ENDIAN
> > + { .format = DRM_FORMAT_XRGB1555 | DRM_FORMAT_BIG_ENDIAN,
> > .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
> > + { .format = DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN, .depth
> > = 16, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
>
> Getting back to the discussion on endianess, I don't understand why the
> BIG_ENDIAN flag is set here. AFAIK these formats are always little
> endian. And the BE flag is set by drivers/userspace if a framebuffer
> has a BE ordering.
>
> It would be better to filter the BE flag in __drm_format_info() before
> the function does the lookup.
I mentioned that alternative in [2], but rejected it because of the
disadvantages:
- {,__}drm_format_info() returns a pointer to a const object,
whose .format field won't have the DRM_FORMAT_BIG_ENDIAN flag set,
complicating callers,
- All callers need to be updated,
- It is difficult to know which big-endian formats are really
supported, especially as only a few are needed.
[2] [PATCH 1/3] drm/fourcc: Add missing big-endian XRGB1555 and RGB565 formats
https://lore.kernel.org/all/0744671ac096a12f0d538906bd324efa71b11400.1657300532.git.ge...@linux-m68k.org
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: [RFC PATCH 1/3] drm: Introduce color fill properties for drm plane
On Wed, 23 Nov 2022 15:27:04 -0800
Jessica Zhang wrote:
> On 11/9/2022 1:18 AM, Pekka Paalanen wrote:
> > On Tue, 8 Nov 2022 23:01:47 +0100
> > Sebastian Wick wrote:
> >
> >> On Tue, Nov 8, 2022 at 7:51 PM Simon Ser wrote:
> >>>
> >>> cc'ing Pekka and wayland-devel for userspace devs feedback on the new
> >>> uAPI.
> >
> > Hi all,
> >
> > thanks! Comments below.
>
> Thanks for the feedback!
>
> >
> >>>
> >>> On Saturday, October 29th, 2022 at 14:08, Dmitry Baryshkov
> >>> wrote:
> >>>
> On 29/10/2022 01:59, Jessica Zhang wrote:
> > Add support for COLOR_FILL and COLOR_FILL_FORMAT properties for
> > drm_plane. In addition, add support for setting and getting the values
> > of these properties.
> >
> > COLOR_FILL represents the color fill of a plane while COLOR_FILL_FORMAT
> > represents the format of the color fill. Userspace can set enable solid
> > fill on a plane by assigning COLOR_FILL to a uint64_t value, assigning
> > the COLOR_FILL_FORMAT property to a uint32_t value, and setting the
> > framebuffer to NULL.
> >
> > Signed-off-by: Jessica Zhang
>
> Planes report supported formats using the drm_mode_getplane(). You'd
> also need to tell userspace, which formats are supported for color fill.
> I don't think one supports e.g. YV12.
>
> A bit of generic comment for the discussion (this is an RFC anyway).
> Using color_fill/color_fill_format properties sounds simple, but this
> might be not generic enough. Limiting color_fill to 32 bits would
> prevent anybody from using floating point formats (e.g.
> DRM_FORMAT_XRGB16161616F, 64-bit value). Yes, this can be solved with
> e.g. using 64-bit for the color_fill value, but then this doesn't sound
> extensible too much.
>
> So, a question for other hardware maintainers. Do we have hardware that
> supports such 'color filled' planes? Do we want to support format
> modifiers for filling color/data? Because what I have in mind is closer
> to the blob structure, which can then be used for filling the plane:
>
> struct color_fill_blob {
> u32 pixel_format;
> u64 modifiers4];
> u32 pixel_data_size; // fixme: is this necessary?
> u8 pixel_data[];
> };
>
> And then... This sounds a lot like a custom framebuffer.
>
> So, maybe what should we do instead is to add new DRM_MODE_FB_COLOR_FILL
> flag to the framebuffers, which would e.g. mean that the FB gets stamped
> all over the plane. This would also save us from changing if (!fb)
> checks all over the drm core.
>
> Another approach might be using a format modifier instead of the FB flag.
>
> What do you think?
> >>>
> >>> First off, we only need to represent the value of a single pixel here. So
> >>> I'm
> >>> not quite following why we need format modifiers. Format modifiers
> >>> describe how
> >>> pixels are laid out in memory. Since there's a single pixel described,
> >>> this
> >>> is non-sensical to me, the format modifier is always LINEAR.
> >
> > Agreed.
> >
> >>>
> >>> Then, I can understand why putting the pixel_format in there is tempting
> >>> to
> >>> guarantee future extensibility, but it also adds complexity. For
> >>> instance, how
> >>> does user-space figure out which formats can be used for COLOR_FILL? Can
> >>> user-space use any format supported by the plane? What does it mean for
> >>> multi-planar formats? Do we really want the kernel to have conversion
> >>> logic for
> >>> all existing formats? Do we need to also add a new read-only blob prop to
> >>> indicate supported COLOR_FILL formats?
>
> FWIW the formats supported by solid_fill wouldn't necessarily be all the
> formats supported by the plane (ex. for msm/dpu, solid_fill only
> supports all RGB color variants, though planes can normally support YUV
> formats too).
>
> That being said, I'm ok with having the solid_fill take in only
> RGBA32323232 format based on the comments below.
>
> >
> > Right. This does not seem to require pixel formats at all.
> >
> > The point of pixel formats is to be able to feed large amounts of data
> > as-is into hardware and avoid the CPU ever touching it. You do that
> > with DRM FBs pointing to suitably allocated hardware buffers. But here
> > we have exactly one pixel, which I imagine will always be read by the
> > CPU so the driver will convert it into a hardware-specific format and
> > program it; probably the driver will not create an internal DRM FB for
> > it. >
> > The above might also be a reason to not model this as a special-case
> > DRM FB in UAPI. Or, at least you need a whole new ioctl to create such
> > DRM FB to avoid the need to allocate e.g. a dumb buffer or a
> > GPU-specific buffer. >
> > What one does need is what Sebastian brought up: does it support alpha
> > or not?
> Hmm, the drm_pla
Re: [PATCH resend v2] drm/fourcc: Add missing big-endian XRGB1555 and RGB565 formats
Hi
Am 23.11.22 um 17:43 schrieb Geert Uytterhoeven:
As of commit eae06120f1974e1a ("drm: refuse ADDFB2 ioctl for broken
bigendian drivers"), drivers must set the
quirk_addfb_prefer_host_byte_order quirk to make the drm_mode_addfb()
compat code work correctly on big-endian machines.
While that works fine for big-endian XRGB and ARGB, which are
mapped to the existing little-endian BGRX and BGRA formats, it
does not work for big-endian XRGB1555 and RGB565, as the latter are not
listed in the format database.
Fix this by adding the missing formats. Limit this to big-endian
platforms, as there is currently no need to support these formats on
little-endian platforms.
Fixes: 6960e6da9cec3f66 ("drm: fix drm_mode_addfb() on big endian machines.")
Signed-off-by: Geert Uytterhoeven
---
v2:
- Use "DRM_FORMAT_foo | DRM_FORMAT_BIG_ENDIAN" instead of
"DRM_FORMAT_HOST_foo",
- Turn into a lone patch, as all other patches from series
https://lore.kernel.org/r/cover.1657300532.git.ge...@linux-m68k.org
were applied to drm-misc/for-linux-next.
---
drivers/gpu/drm/drm_fourcc.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index e09331bb3bc73f21..265671a7f9134c1f 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -190,6 +190,10 @@ const struct drm_format_info *__drm_format_info(u32 format)
{ .format = DRM_FORMAT_BGRA5551,.depth = 15,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
{ .format = DRM_FORMAT_RGB565, .depth = 16,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
{ .format = DRM_FORMAT_BGR565, .depth = 16,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+#ifdef __BIG_ENDIAN
+ { .format = DRM_FORMAT_XRGB1555 | DRM_FORMAT_BIG_ENDIAN, .depth
= 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN, .depth =
16, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
Getting back to the discussion on endianess, I don't understand why the
BIG_ENDIAN flag is set here. AFAIK these formats are always little
endian. And the BE flag is set by drivers/userspace if a framebuffer
has a BE ordering.
It would be better to filter the BE flag in __drm_format_info() before
the function does the lookup.
Best regards
Thomas
+#endif
{ .format = DRM_FORMAT_RGB888, .depth = 24,
.num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 },
{ .format = DRM_FORMAT_BGR888, .depth = 24,
.num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 },
{ .format = DRM_FORMAT_XRGB,.depth = 24,
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
[PULL] drm-intel-fixes
Hi Dave, Daniel, A couple of fixes for 6.1-rc. One TTM backend fix, one display warning fixlet and a merge of two GVT patches which fix KVM reference count handling there. Regards, Tvrtko drm-intel-fixes-2022-11-24: - Fix GVT KVM reference count handling (Sean Christopherson) - Never purge busy TTM objects (Matthew Auld) - Fix warn in intel_display_power_*_domain() functions (Imre Deak) The following changes since commit eb7081409f94a9a8608593d0fb63a1aa3d6f95d8: Linux 6.1-rc6 (2022-11-20 16:02:16 -0800) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2022-11-24 for you to fetch changes up to 14af5d385878d22546914d37f13a314b14825a42: Merge tag 'gvt-fixes-2022-11-11' of https://github.com/intel/gvt-linux into drm-intel-fixes (2022-11-22 07:59:17 +) - Fix GVT KVM reference count handling (Sean Christopherson) - Never purge busy TTM objects (Matthew Auld) - Fix warn in intel_display_power_*_domain() functions (Imre Deak) Imre Deak (1): drm/i915: Fix warn in intel_display_power_*_domain() functions Matthew Auld (1): drm/i915/ttm: never purge busy objects Sean Christopherson (2): drm/i915/gvt: Get reference to KVM iff attachment to VM is successful drm/i915/gvt: Unconditionally put reference to KVM when detaching vGPU Tvrtko Ursulin (1): Merge tag 'gvt-fixes-2022-11-11' of https://github.com/intel/gvt-linux into drm-intel-fixes drivers/gpu/drm/i915/display/intel_display_power.c | 8 drivers/gpu/drm/i915/gem/i915_gem_ttm.c| 4 drivers/gpu/drm/i915/gvt/kvmgt.c | 8 +++- 3 files changed, 11 insertions(+), 9 deletions(-)
Re: [PATCH 00/23] Add Tegra20 parallel video input capture
Hi Luca, On 09/11/2022 15:18, luca.ceres...@bootlin.com wrote: > From: Luca Ceresoli > > Tegra20 and other Tegra SoCs have a video input (VI) peripheral that can > receive from either MIPI CSI-2 or parallel video (called respectively "CSI" > and "VIP" in the documentation). The kernel currently has a staging driver > for Tegra210 CSI capture. This patch set adds support for Tegra20 VIP > capture. > > Unfortunately I had no real documentation available to base this work on, > and I started from a working downstream 3.1 kernel, that I have heavily > reworked to fit into the existing tegra-video driver. The existing code > appears written with the intent of being modular and allow adding new input > mechanisms and new SoCs while keeping a unique VI core module. However its > modularity and extensibility was not enough to add Tegra20 VIP support, so > I added some hooks to turn hard-coded behaviour into per-SoC or per-bus > customizable code. There are also some fixes, some generic cleanups and DT > bindings. I plan on testing this series (esp. making sure it keeps working on non-tegra210 hardware), but I have to find time for that. This is on my TODO list, so I will get to it, but it might time. In the meantime, don't hesitate to post new versions (I saw comments on the bindings), when I have time for testing I'll pick the latest version you post. In other words, don't let lack of review comments from me stop you from working on this :-) Regards, Hans > > Quick tour of the patches: > > * Device tree bindings and minor DTS improvements > >01. dt-bindings: display: tegra: add bindings for Tegra20 VIP >02. dt-bindings: display: tegra: vi: add 'vip' property and example >03. ARM: dts: tegra20: add label to nvidia,tegra20-vi node >04. ARM: dts: tegra20-tamonten: add label to avdd_vdac regulator > > * A fix > >05. staging: media: tegra-video: fix .vidioc_enum_fmt_vid_cap to return > all formats > > * Minor improvements to logging, comments, cleanups > >06. staging: media: tegra-video: improve documentation of > tegra_video_format fields >07. staging: media: tegra-video: document > tegra_channel_get_remote_source_subdev >08. staging: media: tegra-video: fix typos in comment >09. staging: media: tegra-video: improve error messages >10. staging: media: tegra-video: slightly simplify cleanup on errors >11. staging: media: tegra-video: move private struct declaration to C file >12. staging: media: tegra-video: remove unneeded include > > * Preparation to make the VI module generic enough to host Tegra20 and VIP > >13. staging: media: tegra-video: Kconfig: allow TPG only on Tegra210 >14. staging: media: tegra-video: move tegra_channel_fmt_align to a per-soc > op >15. staging: media: tegra-video: move default format to soc-specific data >16. staging: media: tegra-video: move MIPI calibration calls from VI to CSI >17. staging: media: tegra-video: add a per-soc enable/disable op >18. staging: media: tegra-video: move syncpt init/free to a per-soc op >19. staging: media: tegra-video: add syncpts for Tegra20 to struct tegra_vi >20. staging: media: tegra-video: add hooks for planar YUV and H/V flip >21. staging: media: tegra-video: add H/V flip controls > > * Implementation of VIP and Tegra20 > >22. staging: media: tegra-video: add support for VIP (parallel video input) >23. staging: media: tegra-video: add tegra20 variant > > Enjoy! > > Luca > > Luca Ceresoli (23): > dt-bindings: display: tegra: add bindings for Tegra20 VIP > dt-bindings: display: tegra: vi: add 'vip' property and example > ARM: dts: tegra20: add label to nvidia,tegra20-vi node > ARM: dts: tegra20-tamonten: add label to avdd_vdac regulator > staging: media: tegra-video: fix .vidioc_enum_fmt_vid_cap to return > all formats > staging: media: tegra-video: improve documentation of > tegra_video_format fields > staging: media: tegra-video: document > tegra_channel_get_remote_source_subdev > staging: media: tegra-video: fix typos in comment > staging: media: tegra-video: improve error messages > staging: media: tegra-video: slightly simplify cleanup on errors > staging: media: tegra-video: move private struct declaration to C file > staging: media: tegra-video: remove unneeded include > staging: media: tegra-video: Kconfig: allow TPG only on Tegra210 > staging: media: tegra-video: move tegra_channel_fmt_align to a per-soc > op > staging: media: tegra-video: move default format to soc-specific data > staging: media: tegra-video: move MIPI calibration calls from VI to > CSI > staging: media: tegra-video: add a per-soc enable/disable op > staging: media: tegra-video: move syncpt init/free to a per-soc op > staging: media: tegra-video: add syncpts for Tegra20 to struct > tegra_vi > staging: media: tegra-video: add hooks for planar YUV and H/V flip > staging: media: tegra-vid
Re: [PATCH 00/24] drm: Introduce Kunit Tests to VC4
On Wed, Nov 23, 2022 at 11:28 PM Maxime Ripard wrote: > > Hi, > > This series introduce Kunit tests to the vc4 KMS driver, but unlike what we > have been doing so far in KMS, it actually tests the atomic modesetting code. > > In order to do so, I've had to improve a fair bit on the Kunit helpers already > found in the tree in order to register a full blown and somewhat functional > KMS > driver. > > It's of course relying on a mock so that we can test it anywhere. The mocking > approach created a number of issues, the main one being that we need to create > a decent mock in the first place, see patch 22. The basic idea is that I > created some structures to provide a decent approximation of the actual > hardware, and that would support both major architectures supported by vc4. > > This is of course meant to evolve over time and support more tests, but I've > focused on testing the HVS FIFO assignment code which is fairly tricky (and > the > tests have actually revealed one more bug with our current implementation). I > used to have a userspace implementation of those tests, where I would copy and > paste the kernel code and run the tests on a regular basis. It's was obviously > fairly suboptimal, so it seemed like the perfect testbed for that series. > > Let me know what you think, > Maxime > > To: David Airlie > To: Daniel Vetter > To: Maarten Lankhorst > To: Maxime Ripard > To: Thomas Zimmermann > Cc: Dave Stevenson > Cc: Javier Martinez Canillas > Cc: Greg Kroah-Hartman > Cc: Maíra Canal > Cc: Brendan Higgins > Cc: David Gow > Cc: linux-kselft...@vger.kernel.org > Cc: kunit-...@googlegroups.com > Cc: dri-devel@lists.freedesktop.org > Cc: linux-ker...@vger.kernel.org > Cc: linux-me...@vger.kernel.org > Cc: linaro-mm-...@lists.linaro.org > Signed-off-by: Maxime Ripard > > --- Hi Maxime, Thanks very much for this! I'm really excited to see these sorts of tests being written. I was able to successfully run these under qemu with: ./tools/testing/kunit/kunit.py run --kunitconfig drivers/gpu/drm/vc4/tests --arch arm64 --cross_compile=aarch64-linux-gnu- (and also with clang, using --make_options LLVM=1 instead of the --cross_compile flag) On the other hand, they don't compile as a module: ERROR: modpost: missing MODULE_LICENSE() in drivers/gpu/drm/vc4/tests/vc4_mock.o ERROR: modpost: missing MODULE_LICENSE() in drivers/gpu/drm/vc4/tests/vc4_mock_crtc.o ERROR: modpost: missing MODULE_LICENSE() in drivers/gpu/drm/vc4/tests/vc4_mock_output.o ERROR: modpost: missing MODULE_LICENSE() in drivers/gpu/drm/vc4/tests/vc4_mock_plane.o ERROR: modpost: missing MODULE_LICENSE() in drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.o ERROR: modpost: missing MODULE_LICENSE() in drivers/gpu/drm/tests/drm_managed_test.o ERROR: modpost: "vc4_drm_driver" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! ERROR: modpost: "vc5_drm_driver" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! ERROR: modpost: "drm_kunit_helper_alloc_device" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! ERROR: modpost: "__drm_kunit_helper_alloc_drm_device_with_driver" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! ERROR: modpost: "__vc4_hvs_alloc" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! ERROR: modpost: "vc4_dummy_plane" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! ERROR: modpost: "vc4_mock_pv" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! ERROR: modpost: "vc4_dummy_output" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! ERROR: modpost: "vc4_kms_load" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! ERROR: modpost: "vc4_txp_crtc_data" [drivers/gpu/drm/vc4/tests/vc4_mock.ko] undefined! WARNING: modpost: suppressed 17 unresolved symbol warnings because there were too many) Most of those are just the need to export some symbols. There's some work underway to support conditionally exporting symbols only if KUnit is enabled, which may help: https://lore.kernel.org/linux-kselftest/20221102175959.2921063-1-rm...@google.com/ Otherwise, I suspect the better short-term solution would just be to require that the tests are built-in (or at least compiled into whatever of the drm/vc4 modules makes most sense). The only other thing which has me a little confused is the naming of some of the functions, specifically with the __ prefix. Is it just for internal functions (many of them aren't static, but maybe they could use the VISIBLE_IF_KUNIT macro if that makes sense), or for versions of functions which accept extra arguments? Not a big deal (and maybe it's a DRM naming convention I'm ignorant of), but I couldn't quite find a pattern on my first read through. But on the whole, these look good from a KUnit point-of-view. It's really to see some solid mocking and driver testing, too. There would be ways to avoid passing the 'struct kunit' around in more places (or to store extra data as a kunit_resource), but I think it's better overall to pass it around like you have in this case -- it's certainly more compatible with t
Re: [RFC v2 README 00/13] DRM scheduling cgroup controller
Help
Re: [PATCH v4] drm/fourcc: Add Synaptics VideoSmart tiled modifiers
> On Nov 24, 2022, at 12:42 AM, Daniel Vetter wrote:
>
> On Wed, Nov 23, 2022 at 10:58:11PM +0800, Jisheng Zhang wrote:
>>> On Wed, Nov 23, 2022 at 05:19:57PM +0800, Hsia-Jun Li wrote:
>>> From: "Hsia-Jun(Randy) Li"
>>> Memory Traffic Reduction(MTR) is a module in Synaptics
>>> VideoSmart platform could process lossless compression image
>>> and cache the tile memory line.
>>> Those modifiers only record the parameters would effort pixel
>>> layout or memory layout. Whether physical memory page mapping
>>> is used is not a part of format.
>>> We would allocate the same size of memory for uncompressed
>>> and compressed luma and chroma data, while the compressed buffer
>>> would request two extra planes holding the metadata for
>>> the decompression.
>>> Signed-off-by: Hsia-Jun(Randy) Li
>>> ---
>>> include/uapi/drm/drm_fourcc.h | 75 +++
>>> 1 file changed, 75 insertions(+)
>>> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
>>> index bc056f2d537d..ca0b4ca70b36 100644
>>> --- a/include/uapi/drm/drm_fourcc.h
>>> +++ b/include/uapi/drm/drm_fourcc.h
>>> @@ -407,6 +407,7 @@ extern "C" {
>>> #define DRM_FORMAT_MOD_VENDOR_ARM 0x08
>>> #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09
>>> #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a
>>> +#define DRM_FORMAT_MOD_VENDOR_SYNAPTICS 0x0b
>> Any users in the mainline tree?
Not yet. I believe a V4L2 codec would be the first one.
Still there are many patches are requested for v4l2 which currently does not
support format modifier. You could find discussion in linux media list.
This does need the agreement from drm maintainers, three of us tend to drop the
pixel formats in video4linux2.h only keeping those codec formats in new
extended v4l2 format negotiation interface. All the pixel formats should go to
drm_fourcc.h while we can’t decide how to present those hardware requests
contiguous memory.
We don’t bring those NV12M into drm_fourcc.h, we hate that.
> Note that drm_fourcc.h serves as the vendor-neutral registry for these
> numbers, and they're referenced in both gl and vk extensions. So this is
> the one case where we do _not_ require in-kernel users or open source
> userspace.
>
The first user for these pixel formats would be the software pixel reader for
Gstreamer, I am planning to add the unpacker for the two uncompressed pixel
formats.
> If there is someone interested in an in-kernel or open userspace driver
> though it would be really great to have their acks before merging. Just to
> make sure that the modifiers will work with both upstream and downstream
> driver stacks.
This patch have been reviewed internally, it is good enough to describe our
pixel formats.
>
> I just realized that we've failed to document this, I'll type up a patch.
About the format itself, I have sent the document to the mesa, you could find a
MR there.
> -Daniel
>
>
>>> /* add more to the end as needed */
>>> @@ -1507,6 +1508,80 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64
>>> modifier)
>>> #define AMD_FMT_MOD_CLEAR(field) \
>>> (~((__u64)AMD_FMT_MOD_##field##_MASK << AMD_FMT_MOD_##field##_SHIFT))
>>> +/*
>>> + * Synaptics VideoSmart modifiers
>>> + *
>>> + * Tiles could be arranged in Groups of Tiles (GOTs), it is a small tile
>>> + * within a tile. GOT size and layout varies based on platform and
>>> + * performance concern. When the compression is applied, it is possible
>>> + * that we would have two tile type in the GOT, these parameters can't
>>> + * tell the secondary tile type.
>>> + *
>>> + * Besides, an 8 size 4 bytes arrary (32 bytes) would be need to store
>>> + * some compression parameters for a compression meta data plane.
>>> + *
>>> + * Macro
>>> + * Bits Param Description
>>> + * -
>>> -
>>> + *
>>> + * 7:0 f Scan direction description.
>>> + *
>>> + * 0 = Invalid
>>> + * 1 = V4, the scan would always start from vertical for 4
>>> pixel
>>> + * then move back to the start pixel of the next
>>> horizontal
>>> + * direction.
>>> + * 2 = Reserved for future use.
>>> + *
>>> + * 15:8 m The times of pattern repeat in the right angle direction
>>> from
>>> + * the first scan direction.
>>> + *
>>> + * 19:16 p The padding bits after the whole scan, could be zero.
>>> + *
>>> + * 20:20 g GOT packing flag.
>>> + *
>>> + * 23:21 - Reserved for future use. Must be zero.
>>> + *
>>> + * 27:24 h log2(horizontal) of bytes, in GOTs.
>>> + *
>>> + * 31:28 v log2(vertical) of bytes, in GOTs.
>>> + *
>>> + * 35:32 - Reserved for future use. Must be zero.
>>> + *
>>> + * 36:36 c Compression flag.
>>> + *
>>> + * 55:37 - Reserved for future use. Must be zero.
>>> + *
>>> + */
>>> +
>>> +#define DRM_FORMAT_MOD_SYNA_V4_TILEDfourcc_mod_code(SYNAPTICS, 1)
>>> +
>>> +#define DRM_FORMAT_
[PATCH] swsmu/amdgpu_smu: Fix the wrong if-condition
The logical operator '&&' will make
smu->ppt_funcs->set_gfx_power_up_by_imu segment fault when
smu->ppt_funcs is NULL.
Signed-off-by: Yu Songping
---
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index b880f4d7d67e..1cb728b0b7ee 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -161,7 +161,7 @@ int smu_get_dpm_freq_range(struct smu_context *smu,
int smu_set_gfx_power_up_by_imu(struct smu_context *smu)
{
- if (!smu->ppt_funcs && !smu->ppt_funcs->set_gfx_power_up_by_imu)
+ if (!smu->ppt_funcs || !smu->ppt_funcs->set_gfx_power_up_by_imu)
return -EOPNOTSUPP;
return smu->ppt_funcs->set_gfx_power_up_by_imu(smu);
--
2.17.1
Re: [PATCH v4] drm/fourcc: Add Synaptics VideoSmart tiled modifiers
> On Nov 24, 2022, at 1:27 AM, Daniel Vetter wrote:
>
> CAUTION: Email originated externally, do not click links or open attachments
> unless you recognize the sender and know the content is safe.
>
>
>> On Thu, Nov 24, 2022 at 01:14:48AM +0800, Randy Li wrote:
>>
On Nov 24, 2022, at 12:42 AM, Daniel Vetter wrote:
>>>
>>> On Wed, Nov 23, 2022 at 10:58:11PM +0800, Jisheng Zhang wrote:
> On Wed, Nov 23, 2022 at 05:19:57PM +0800, Hsia-Jun Li wrote:
> From: "Hsia-Jun(Randy) Li"
> Memory Traffic Reduction(MTR) is a module in Synaptics
> VideoSmart platform could process lossless compression image
> and cache the tile memory line.
> Those modifiers only record the parameters would effort pixel
> layout or memory layout. Whether physical memory page mapping
> is used is not a part of format.
> We would allocate the same size of memory for uncompressed
> and compressed luma and chroma data, while the compressed buffer
> would request two extra planes holding the metadata for
> the decompression.
> Signed-off-by: Hsia-Jun(Randy) Li
> ---
> include/uapi/drm/drm_fourcc.h | 75 +++
> 1 file changed, 75 insertions(+)
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index bc056f2d537d..ca0b4ca70b36 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -407,6 +407,7 @@ extern "C" {
> #define DRM_FORMAT_MOD_VENDOR_ARM 0x08
> #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09
> #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a
> +#define DRM_FORMAT_MOD_VENDOR_SYNAPTICS 0x0b
Any users in the mainline tree?
>> Not yet. I believe a V4L2 codec would be the first one.
>> Still there are many patches are requested for v4l2 which currently does
>> not support format modifier. You could find discussion in linux media
>> list.
>>
>> This does need the agreement from drm maintainers, three of us tend to
>> drop the pixel formats in video4linux2.h only keeping those codec
>> formats in new extended v4l2 format negotiation interface. All the pixel
>> formats should go to drm_fourcc.h while we can’t decide how to present
>> those hardware requests contiguous memory.
>
> Uh no.
>
> These enums are maintained in drm_fourcc.h, by drm maintainers. You
> _cannot_ mix them up with the fourcc enums that video4linux2.h has, that's
> a completely different enum space because fourcc codes are _not_ a
> standard.
>
Things we try to solve is the those non contiguous memory planes in v4l2, we
don’t want to increase them anymore. Besides the values for pixel formats are
the same between V4L2 and drm.
> Please do not ever mix up drm_fourcc format modifiers with v4l2 fourcc
> codes, that will result in complete chaos. There's a reason why there's
> only one authoritative source for these.
>
In the previous version, it would fail in building, because a driver’s
header(ipu-v3) would included both v4l2 and drm. I can’t add another format
modifier macro to v4l2.
If drm doesn’t like the idea that v4l2 use the fourcc from drm, I should inform
people about that.
>> We don’t bring those NV12M into drm_fourcc.h, we hate that.
>>> Note that drm_fourcc.h serves as the vendor-neutral registry for these
>>> numbers, and they're referenced in both gl and vk extensions. So this is
>>> the one case where we do _not_ require in-kernel users or open source
>>> userspace.
>>>
>> The first user for these pixel formats would be the software pixel reader
>> for Gstreamer, I am planning to add the unpacker for the two uncompressed
>> pixel formats.
>>> If there is someone interested in an in-kernel or open userspace driver
>>> though it would be really great to have their acks before merging. Just to
>>> make sure that the modifiers will work with both upstream and downstream
>>> driver stacks.
>> This patch have been reviewed internally, it is good enough to describe our
>> pixel formats.
>>>
>>> I just realized that we've failed to document this, I'll type up a patch.
>>
>> About the format itself, I have sent the document to the mesa, you could
>> find a MR there.
>
> Please include the link to that MR in the patch description.
mesa !19921
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/19921
The reason I didn’t do that is I didn’t get response from mesa.
> -Daniel
>
>>> -Daniel
>>>
>>>
> /* add more to the end as needed */
> @@ -1507,6 +1508,80 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64
> modifier)
> #define AMD_FMT_MOD_CLEAR(field) \
> (~((__u64)AMD_FMT_MOD_##field##_MASK << AMD_FMT_MOD_##field##_SHIFT))
> +/*
> + * Synaptics VideoSmart modifiers
> + *
> + * Tiles could be arranged in Groups of Tiles (GOTs), it is a small tile
> + * within a tile. GOT size and layout varies based on platform and
> + * performance concern. When the compression is applied, it is possible
> + *
Re: [PATCH] dt-bindings: display: bridge: renesas,rzg2l-mipi-dsi: Document RZ/V2L support
On Tue, Nov 22, 2022 at 8:54 PM Biju Das wrote:
> Document RZ/V2L DSI bindings. RZ/V2L MIPI DSI is identical to one found on
> the RZ/G2L SoC. No driver changes are required as generic compatible
> string "renesas,rzg2l-mipi-dsi" will be used as a fallback.
>
> Signed-off-by: Biju Das
Reviewed-by: Geert Uytterhoeven
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: A collection of typo and documentation fixes
Hi T.J,
On Wed, Nov 23, 2022 at 07:35:18PM +, T.J. Mercier wrote:
> I've been collecting these typo fixes for a while and it feels like
> time to send them in.
>
> Signed-off-by: T.J. Mercier
> ---
> drivers/dma-buf/dma-buf.c | 14 +++---
> include/linux/dma-buf.h | 6 +++---
> 2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index dd0f83ee505b..614ccd208af4 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -1141,7 +1141,7 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_unmap_attachment, DMA_BUF);
> *
> * @dmabuf: [in]buffer which is moving
> *
> - * Informs all attachmenst that they need to destroy and recreated all their
> + * Informs all attachments that they need to destroy and recreate all their
> * mappings.
> */
> void dma_buf_move_notify(struct dma_buf *dmabuf)
> @@ -1159,11 +1159,11 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_move_notify, DMA_BUF);
> /**
> * DOC: cpu access
> *
> - * There are mutliple reasons for supporting CPU access to a dma buffer
> object:
> + * There are multiple reasons for supporting CPU access to a dma buffer
> object:
> *
> * - Fallback operations in the kernel, for example when a device is
> connected
> * over USB and the kernel needs to shuffle the data around first before
> - * sending it away. Cache coherency is handled by braketing any
> transactions
> + * sending it away. Cache coherency is handled by bracketing any
> transactions
> * with calls to dma_buf_begin_cpu_access() and dma_buf_end_cpu_access()
> * access.
> *
> @@ -1190,7 +1190,7 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_move_notify, DMA_BUF);
> * replace ION buffers mmap support was needed.
> *
> * There is no special interfaces, userspace simply calls mmap on the
> dma-buf
> - * fd. But like for CPU access there's a need to braket the actual access,
> + * fd. But like for CPU access there's a need to bracket the actual access,
> * which is handled by the ioctl (DMA_BUF_IOCTL_SYNC). Note that
> * DMA_BUF_IOCTL_SYNC can fail with -EAGAIN or -EINTR, in which case it
> must
> * be restarted.
> @@ -1264,10 +1264,10 @@ static int __dma_buf_begin_cpu_access(struct dma_buf
> *dmabuf,
> * preparations. Coherency is only guaranteed in the specified range for the
> * specified access direction.
> * @dmabuf: [in]buffer to prepare cpu access for.
> - * @direction: [in]length of range for cpu access.
> + * @direction: [in]direction of access.
> *
> * After the cpu access is complete the caller should call
> - * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls
> is
> + * dma_buf_end_cpu_access(). Only when cpu access is bracketed by both calls
> is
> * it guaranteed to be coherent with other DMA access.
> *
> * This function will also wait for any DMA transactions tracked through
> @@ -1307,7 +1307,7 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_begin_cpu_access, DMA_BUF);
> * actions. Coherency is only guaranteed in the specified range for the
> * specified access direction.
> * @dmabuf: [in]buffer to complete cpu access for.
> - * @direction: [in]length of range for cpu access.
> + * @direction: [in]direction of access.
> *
> * This terminates CPU access started with dma_buf_begin_cpu_access().
> *
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index 71731796c8c3..1d61a4f6db35 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -330,7 +330,7 @@ struct dma_buf {
>* @lock:
>*
>* Used internally to serialize list manipulation, attach/detach and
> - * vmap/unmap. Note that in many cases this is superseeded by
> + * vmap/unmap. Note that in many cases this is superseded by
>* dma_resv_lock() on @resv.
>*/
> struct mutex lock;
> @@ -365,7 +365,7 @@ struct dma_buf {
>*/
> const char *name;
>
> - /** @name_lock: Spinlock to protect name acces for read access. */
> + /** @name_lock: Spinlock to protect name access for read access. */
> spinlock_t name_lock;
>
> /**
> @@ -402,7 +402,7 @@ struct dma_buf {
>* anything the userspace API considers write access.
>*
>* - Drivers may just always add a write fence, since that only
> - * causes unecessarily synchronization, but no correctness issues.
> + * causes unnecessary synchronization, but no correctness issues.
>*
>* - Some drivers only expose a synchronous userspace API with no
>* pipelining across drivers. These do not set any fences for their
> --
> 2.38.1.584.g0f3c55d4c2-goog
>
Looks good to me.
Reviewed-by: Tommaso Merciai
Thanks & Regards,
Tommaso
--
Tommaso Merciai
Embedded Linux Engineer
tommaso.merc...@amarulasolutions.com
__
Amarula Solutions SRL
Via Le Canevare 30, 31100 Trevis
Re: [PATCH resend v2] drm/fourcc: Add missing big-endian XRGB1555 and RGB565 formats
Hi Gerd,
On Thu, Nov 24, 2022 at 8:29 AM Gerd Hoffmann wrote:
> > > > +#ifdef __BIG_ENDIAN
> > >
> > > Why do we need the #ifdef here? Iirc some hw has big endian flags in the
> > > scanout registers, so could supprt this unconditionally if there's no
> > > #ifdef around the format defines. Some drivers might then also want a
> > > DRM_FORMAT_FOO_BE define to simplify tables and stuff, but that's more a
> > > bikeshed.
> >
> > "Limit this to big-endian platforms, as there is currently no need
> > to support these formats on little-endian platforms."
> >
> > Will anyone make use of this? In theory, all of the 16-bpp formats
> > can have a big-endian counterpart.
>
> Highly unlikely. Dealing with 16-bpp formats in non-native byte order
> is a PITA because it isn't enough to simply adjust the masks and shifts
> to pick the correct bits and be done with it.
>
> The qemu stdvga happens to have a register to switch framebuffer
> byteorder (so both x64 and ppc are happy), and the bochs drm driver
> actually supports that no matter what the cpu byte order is, but it
> supports only DRM_FORMAT_XRGB + DRM_FORMAT_BGRX.
>
> Supporting 16 bpp in the driver wouldn't be that much of a problem, but
> processing the framebuffer on the host side when emulating a big endian
> guest on a little endian host is painful. I think I can't ask pixman to
> do a conversation from DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN to
> DRM_FORMAT_XRGB on a little endian machine.
Indeed. But you can do a quick 16-bit byteswap, and convert from
DRM_FORMAT_RGB565 to DRM_FORMAT_XRGB?
There's a similar issue with Cairo, cfr. '[PATCH libdrm v2 08/10]
util: Fix pwetty on big-endian"[1].
BTW, does pixman support converting DRM_FORMAT_RGB565 to
DRM_FORMAT_XRGB on a big-endian machine?
[1]
https://lore.kernel.org/all/e8597038478f12e9eda5e86b309b52988f69f2eb.1657302103.git.ge...@linux-m68k.org
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
