Re: [PATCH v2, 1/1] mailbox: cmdq: add instruction time-out interrupt support
On Fri, 2021-10-08 at 22:55 +0800, Chun-Kuang Hu wrote: > Hi, Yongqiang: > > yongqiang.niu 於 2021年10月8日 週五 上午9:49寫道: > > > > On Tue, 2021-10-05 at 07:41 +0800, Chun-Kuang Hu wrote: > > > Hi, Yongqiang: > > > > > > Yongqiang Niu 於 2021年9月30日 週四 > > > 下午9:18寫道: > > > > > > > > add time-out cycle setting to make sure time-out interrupt irq > > > > will happened when instruction time-out for wait and poll > > > > > > > > Signed-off-by: Yongqiang Niu > > > > --- > > > > drivers/mailbox/mtk-cmdq-mailbox.c | 11 +++ > > > > 1 file changed, 11 insertions(+) > > > > > > > > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c > > > > b/drivers/mailbox/mtk-cmdq-mailbox.c > > > > index 64175a893312..197b03222f94 100644 > > > > --- a/drivers/mailbox/mtk-cmdq-mailbox.c > > > > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c > > > > @@ -36,6 +36,7 @@ > > > > #define CMDQ_THR_END_ADDR 0x24 > > > > #define CMDQ_THR_WAIT_TOKEN0x30 > > > > #define CMDQ_THR_PRIORITY 0x40 > > > > +#define CMDQ_THR_INSTN_TIMEOUT_CYCLES 0x50 > > > > > > > > #define GCE_GCTL_VALUE 0x48 > > > > > > > > @@ -54,6 +55,15 @@ > > > > #define CMDQ_JUMP_BY_OFFSET0x1000 > > > > #define CMDQ_JUMP_BY_PA0x1001 > > > > > > > > +/* > > > > + * instruction time-out > > > > + * cycles to issue instruction time-out interrupt for wait and > > > > poll instructions > > > > + * GCE axi_clock 156MHz > > > > + * 1 cycle = 6.41ns > > > > + * instruction time out 2^22*2*6.41ns = 53ms > > > > > > For different clients, the timeout value would be different, and > > > each > > > client could use timer to detect timeout, so it's not necessary > > > to > > > enable timeout in cmdq driver. > > > > > > Regards, > > > Chun-Kuang. > > > > if we do not set cmdq hardware timeout, this condition will never > > happen > > cmdq_thread_irq_handler > > if (irq_flag & CMDQ_THR_IRQ_ERROR) > > err = true; > > > > and no error callback > > else if (err) { > > cmdq_task_exec_done(task, -ENOEXEC); > > cmdq_task_handle_error(curr_task); > > kfree(task); > > } > > the client will never received the error callback, cmdq hardware > > will > > poll the event for ever and no report timeout > > I think there are two way to implement the timeout mechanism. The > first way is to use the GCE hardware to detect timeout. The second > way > is that client driver use timer to detect timeout, when it's timeout, > use mbox_flush() to clean up the packets in mtk cmdq driver, and > remove the error handle in irq handler. > If you think the first way is better, I think you should pass the > timeout value from client driver to cmdq driver because each client > driver has different timeout value. And the GCE clock may be > different > in each SoC, so use clk_get_rate() to get the clock frequency for > different SoC. > > Regards, > Chun-Kuang. this patch only used for mdp instruction error and and trigger hw timeout flow, it is for debug, i will abandon this patch and will not send upstream anymore. > > > > > > > > + */ > > > > +#define CMDQ_INSTN_TIMEOUT_CYCLES 22 > > > > + > > > > struct cmdq_thread { > > > > struct mbox_chan*chan; > > > > void __iomem*base; > > > > @@ -376,6 +386,7 @@ static int cmdq_mbox_send_data(struct > > > > mbox_chan > > > > *chan, void *data) > > > > writel((task->pa_base + pkt->cmd_buf_size) >> > > > > cmdq- > > > > > shift_pa, > > > > > > > >thread->base + CMDQ_THR_END_ADDR); > > > > > > > > + writel(CMDQ_INSTN_TIMEOUT_CYCLES, thread->base > > > > + > > > > CMDQ_THR_INSTN_TIMEOUT_CYCLES); > > > > writel(thread->priority, thread->base + > > > > CMDQ_THR_PRIORITY); > > > > writel(CMDQ_THR_IRQ_EN, thread->base + > > > > CMDQ_THR_IRQ_ENABLE); > > > > writel(CMDQ_THR_ENABLED, thread->base + > > > > CMDQ_THR_ENABLE_TASK); > > > > -- > > > > 2.25.1 > > > >
Re: [PATCH] drm/scheduler: fix drm_sched_job_add_implicit_dependencies harder
Am 16.11.21 um 19:30 schrieb Amit Pundir: On Tue, 16 Nov 2021 at 21:21, Rob Clark wrote: From: Rob Clark drm_sched_job_add_dependency() could drop the last ref, so we need to do the dma_fence_get() first. It fixed the splats I saw on RB5 (sm8250 | A650). Thanks. Tested-by: Amit Pundir I've added my rb, pushed this with the original fix to drm-misc-fixes and cleaned up the obvious fallout between drm-misc-fixes and drm-misc-next in drm-tip. Thanks for the help and sorry for the noise, Christian. Cc: Christian König Fixes: 9c2ba265352a drm/scheduler: ("use new iterator in drm_sched_job_add_implicit_dependencies v2") Signed-off-by: Rob Clark --- Applies on top of "drm/scheduler: fix drm_sched_job_add_implicit_dependencies" but I don't think that has a stable commit sha yet. drivers/gpu/drm/scheduler/sched_main.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 94fe51b3caa2..f91fb31ab7a7 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -704,12 +704,13 @@ int drm_sched_job_add_implicit_dependencies(struct drm_sched_job *job, int ret; dma_resv_for_each_fence(, obj->resv, write, fence) { - ret = drm_sched_job_add_dependency(job, fence); - if (ret) - return ret; - /* Make sure to grab an additional ref on the added fence */ dma_fence_get(fence); + ret = drm_sched_job_add_dependency(job, fence); + if (ret) { + dma_fence_put(fence); + return ret; + } } return 0; } -- 2.33.1
[Bug 201957] amdgpu: ring gfx timeout
https://bugzilla.kernel.org/show_bug.cgi?id=201957 Fusion Future (qydwhotm...@gmail.com) changed: What|Removed |Added CC||qydwhotm...@gmail.com --- Comment #49 from Fusion Future (qydwhotm...@gmail.com) --- Ryzen 4700U same error. openSUSE Tumbleweed X11 Kernel version is 5.14.14 Mesa version is 21.2.5-293.2 Firmware version is 20211027-1.1 -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH v7 20/20] drm/mediatek: add mediatek-drm of vdosys1 support for MT8195
Hi Chun-Kuang, Thanks for the review. On Fri, 2021-11-12 at 07:30 +0800, Chun-Kuang Hu wrote: > Hi, Nancy: > > Nancy.Lin 於 2021年10月29日 週五 下午3:52寫道: > > > > Add driver data of mt8195 vdosys1 to mediatek-drm and the sub > > driver. > > > > Signed-off-by: Nancy.Lin > > --- > > drivers/gpu/drm/mediatek/mtk_disp_merge.c | 4 ++ > > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 13 ++--- > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 30 +-- > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 + > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 56 - > > > > 5 files changed, 78 insertions(+), 26 deletions(-) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_merge.c > > b/drivers/gpu/drm/mediatek/mtk_disp_merge.c > > index dff2797a2f68..d64846c38fe1 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_disp_merge.c > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_merge.c > > @@ -8,6 +8,7 @@ > > #include > > #include > > #include > > +#include > > #include > > > > #include "mtk_drm_ddp_comp.h" > > @@ -79,6 +80,9 @@ void mtk_merge_stop(struct device *dev) > > struct mtk_disp_merge *priv = dev_get_drvdata(dev); > > > > mtk_merge_stop_cmdq(dev, NULL); > > + > > + if (priv->async_clk) > > + device_reset_optional(dev); > > Separate this to an merge patch. > OK. > > } > > > > void mtk_merge_start_cmdq(struct device *dev, struct cmdq_pkt > > *cmdq_pkt) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > index 25580106a2c4..d41bd8201371 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > @@ -876,15 +876,10 @@ int mtk_drm_crtc_create(struct drm_device > > *drm_dev, > > node = priv->comp_node[comp_id]; > > comp = >ddp_comp[comp_id]; > > > > - if (!node) { > > - dev_info(dev, > > -"Not creating crtc %d because > > component %d is disabled or missing\n", > > -crtc_i, comp_id); > > - return 0; > > - } > > - > > - if (!comp->dev) { > > - dev_err(dev, "Component %pOF not > > initialized\n", node); > > + if (!node && !comp->dev) { > > + dev_err(dev, > > + "Not creating crtc %d because > > component %d is disabled, missing or not initialized\n", > > + crtc_i, comp_id); > > Why do this? If this is necessary, separate this to an independent > patch. This is a necessary modification for ovl_adaptor component. Ovl_adaptor is brought up by drm driver, no dts device node for it. To modify the check missing component logic. > > > return -ENODEV; > > } > > } > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > index eb9835102d79..279087ae889b 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > @@ -385,6 +385,18 @@ static const struct mtk_ddp_comp_funcs > > ddp_ufoe = { > > .start = mtk_ufoe_start, > > }; > > > > +static const struct mtk_ddp_comp_funcs ddp_ovl_adaptor = { > > + .clk_enable = mtk_ovl_adaptor_clk_enable, > > + .clk_disable = mtk_ovl_adaptor_clk_disable, > > + .config = mtk_ovl_adaptor_config, > > + .start = mtk_ovl_adaptor_start, > > + .stop = mtk_ovl_adaptor_stop, > > + .layer_nr = mtk_ovl_adaptor_layer_nr, > > + .layer_config = mtk_ovl_adaptor_layer_config, > > + .enable_vblank = mtk_ovl_adaptor_enable_vblank, > > + .disable_vblank = mtk_ovl_adaptor_disable_vblank, > > +}; > > Separate this to an ovl_adaptor patch. > OK > > + > > static const char * const mtk_ddp_comp_stem[MTK_DDP_COMP_TYPE_MAX] > > = { > > [MTK_DISP_AAL] = "aal", > > [MTK_DISP_BLS] = "bls", > > @@ -398,6 +410,7 @@ static const char * const > > mtk_ddp_comp_stem[MTK_DDP_COMP_TYPE_MAX] = { > > [MTK_DISP_OD] = "od", > > [MTK_DISP_OVL] = "ovl", > > [MTK_DISP_OVL_2L] = "ovl-2l", > > + [MTK_DISP_OVL_ADAPTOR] = "ovl_adaptor", > > [MTK_DISP_POSTMASK] = "postmask", > > [MTK_DISP_PWM] = "pwm", > > [MTK_DISP_RDMA] = "rdma", > > @@ -443,6 +456,7 @@ static const struct mtk_ddp_comp_match > > mtk_ddp_matches[DDP_COMPONENT_ID_MAX] = { > > [DDP_COMPONENT_OVL_2L0] = { MTK_DISP_OVL_2L,0, > > _ovl }, > > [DDP_COMPONENT_OVL_2L1] = { MTK_DISP_OVL_2L,1, > > _ovl }, > > [DDP_COMPONENT_OVL_2L2] = { MTK_DISP_OVL_2L,2, > > _ovl }, > > + [DDP_COMPONENT_OVL_ADAPTOR] = { > > MTK_DISP_OVL_ADAPTOR, 0, _ovl_adaptor }, > > [DDP_COMPONENT_POSTMASK0] = { MTK_DISP_POSTMASK, 0, > > _postmask },
[Bug 214991] VC4 DRM waiting for flip down makes UI freeze a while with kernel 5.15
https://bugzilla.kernel.org/show_bug.cgi?id=214991 --- Comment #1 from Jian-Hong Pan (j...@endlessos.org) --- Created attachment 299603 --> https://bugzilla.kernel.org/attachment.cgi?id=299603=edit Full dmesg log with the patch series "drm/vc4: kms: Misc fixes for HVS commits" Thanks to Maxime's reply from https://lore.kernel.org/lkml/2025113348.aylgkwrmaaomqrp4@gilmour/T/ So, I applied the patch series "drm/vc4: kms: Misc fixes for HVS commits" [1] based on latest mainline kernel at commit 8ab774587903 ("Merge tag 'trace-v5.16-5' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace"), which almost equals "tags/v5.16-rc1". However, the system hangs and becomes dead at the kernel message: [drm] Initialized vc4 0.0.0 20140616 for gpu on minor 0 If I revert the patch series to the original mainline kernel, system can boot up. [1]: https://lore.kernel.org/dri-devel/2025113105.103275-1-max...@cerno.tech/ -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
RE: [PATCH v3 0/4] Add Aspeed AST2600 soc display support
Hi Joel, Got it. I will change user name for next patch send. The ast2600 is tested on my platform. And I will try this on the ast2500. Below is testing steps: 1. Apply the patch into project. 2. Add below config for VT and LOGO on. CONFIG_TTY=y CONFIG_VT=y CONFIG_CONSOLE_TRANSLATIONS=y CONFIG_VT_CONSOLE=y CONFIG_VT_CONSOLE_SLEEP=y CONFIG_HW_CONSOLE=y CONFIG_VT_HW_CONSOLE_BINDING=y CONFIG_UNIX98_PTYS=y CONFIG_LDISC_AUTOLOAD=y CONFIG_DEVMEM=y CONFIG_DUMMY_CONSOLE=y CONFIG_FRAMEBUFFER_CONSOLE=y CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY=y CONFIG_LOGO=y CONFIG_LOGO_LINUX_CLUT224=y 3. The Linux logo will be shown on the screen, when the BMC boot in Linux. Thanks, By Tommy > -Original Message- > From: Joel Stanley > Sent: Wednesday, November 17, 2021 2:06 PM > To: Tommy Huang > Cc: David Airlie ; Daniel Vetter ; Rob > Herring ; Andrew Jeffery ; > linux-aspeed ; open list:DRM DRIVERS > ; devicetree ; > Linux ARM ; Linux Kernel Mailing List > ; BMC-SW > Subject: Re: [PATCH v3 0/4] Add Aspeed AST2600 soc display support > > On Wed, 17 Nov 2021 at 05:45, tommy-huang > wrote: > > > > v3: > > Refine the patch for clear separate purpose. > > Skip to send devicetree patch > > Thanks Tommy. A few things: > > - Set up your authorship in git: > > git config --global user.name "Tommy Haung" > > - The "Add AST2600 chip support" patch is the same as the one I sent, you can > put mine back in your series now > > - We should add a device tree bindings document > > Can you confirm you tested these changes on both the ast2500 and the > ast2600? How did you test? > > Cheers, > > Joel > > > > > > v2: > > Remove some unnecessary patch. > > Refine for reviwer request. > > > > v1: > > First add patch. > > > > Joel Stanley (2): > > ARM: dts: aspeed: Add GFX node to AST2600 > > ARM: dts: aspeed: ast2600-evb: Enable GFX device > > > > tommy-huang (2): > > drm/aspeed: Update INTR_STS handling > > drm/aspeed: Add AST2600 chip support > > > > arch/arm/boot/dts/aspeed-ast2600-evb.dts | 18 ++ > > arch/arm/boot/dts/aspeed-g6.dtsi | 11 +++ > > drivers/gpu/drm/aspeed/aspeed_gfx.h | 1 + > > drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 15 ++- > > 4 files changed, 44 insertions(+), 1 deletion(-) > > > > -- > > 2.17.1 > >
Re: [PATCH v1 3/3] phy: qcom: Program SSC only if supported by sink
Quoting Sankeerth Billakanti (2021-11-15 23:07:38) > Some legacy eDP sinks may not support SSC. The support for SSC is > indicated through an opts flag from the controller driver. This > change will enable SSC only if the sink supports it. > > Signed-off-by: Sankeerth Billakanti > --- I suppose as long as the existing user has already chosen to set the opts in the controller driver then this is fine. Reviewed-by: Stephen Boyd
Re: [PATCH v1 2/3] phy: qcom: Add support for eDP PHY on sc7280
Quoting Sankeerth Billakanti (2021-11-15 23:07:37) > The sc7280 platform supports native eDP controller and PHY. > This change will add support for the eDP PHY on sc7280. > > Signed-off-by: Sankeerth Billakanti > --- Reviewed-by: Stephen Boyd
Re: [PATCH v1 1/3] dt-bindings: phy: Add eDP PHY compatible for sc7280
Quoting Sankeerth Billakanti (2021-11-15 23:07:36) > Add compatible string for the supported eDP PHY on sc7280 platform. > > Signed-off-by: Sankeerth Billakanti > --- Reviewed-by: Stephen Boyd
Fix drm suspend and resume issue
Subject: [PATCH 0/3] Fix drm suspend and resume issue 1. When the hardwire components on crtc is using GCE to configure their register, cmdq_suspend may be called. So add cmdq_mbox_flush to clear all tasks and release GCE clocks before cmdq_suspend. 2. The suspend and resume order of components on crtc can be guaranteed by the same power-domain, but cmdq can not. So add devlink to cmdq dev make sure the order of suspend and resume: cmdq_suspend is latter than drm_suspend and cmdq_resume is earlier than drm_resume. jason-jh.lin (3): mialbox: move cmdq suspend,resume and remove after cmdq_mbox_flush mailbox: add cmdq_mbox_flush to clear all task before suspend drm/mediatek: add devlink to cmdq dev drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 13 drivers/mailbox/mtk-cmdq-mailbox.c | 81 - 2 files changed, 52 insertions(+), 42 deletions(-) -- 2.18.0
[PATCH 2/3] mailbox: add cmdq_mbox_flush to clear all task before suspend
CMDQ driver will occupy GCE clock to execute the task in GCE thread. So call cmdq_mbox_flush to clear all task in GCE thread before CMDQ suspend. Signed-off-by: jason-jh.lin --- drivers/mailbox/mtk-cmdq-mailbox.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c index 03f9ed4c5131..28cadfc0091b 100644 --- a/drivers/mailbox/mtk-cmdq-mailbox.c +++ b/drivers/mailbox/mtk-cmdq-mailbox.c @@ -484,21 +484,18 @@ static int cmdq_suspend(struct device *dev) struct cmdq *cmdq = dev_get_drvdata(dev); struct cmdq_thread *thread; int i; - bool task_running = false; cmdq->suspended = true; for (i = 0; i < cmdq->thread_nr; i++) { thread = >thread[i]; if (!list_empty(>task_busy_list)) { - task_running = true; - break; + /* try to clear all task in this thread */ + cmdq_mbox_flush(thread->chan, 2000); + dev_warn(dev, "thread[%d] exist running task(s) in suspend\n", i); } } - if (task_running) - dev_warn(dev, "exist running task(s) in suspend\n"); - clk_bulk_unprepare(cmdq->gce_num, cmdq->clocks); return 0; -- 2.18.0
[PATCH 3/3] drm/mediatek: add devlink to cmdq dev
Add devlink to cmdq to make sure the order of suspend and resume is correct. Signed-off-by: jason-jh.lin --- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index 24d9bde4d6e2..0a472719709d 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -59,6 +59,7 @@ struct mtk_drm_crtc { #endif struct device *mmsys_dev; + struct device *drm_dev; struct mtk_mutex*mutex; unsigned intddp_comp_nr; struct mtk_ddp_comp **ddp_comp; @@ -158,6 +159,7 @@ static void mtk_drm_crtc_destroy(struct drm_crtc *crtc) mtk_drm_cmdq_pkt_destroy(_crtc->cmdq_handle); if (mtk_crtc->cmdq_client.chan) { + device_link_remove(mtk_crtc->drm_dev, mtk_crtc->cmdq_client.chan->mbox->dev); mbox_free_channel(mtk_crtc->cmdq_client.chan); mtk_crtc->cmdq_client.chan = NULL; } @@ -888,6 +890,7 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev, return -ENOMEM; mtk_crtc->mmsys_dev = priv->mmsys_dev; + mtk_crtc->drm_dev = dev; mtk_crtc->ddp_comp_nr = path_len; mtk_crtc->ddp_comp = devm_kmalloc_array(dev, mtk_crtc->ddp_comp_nr, sizeof(*mtk_crtc->ddp_comp), @@ -956,6 +959,16 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev, } if (mtk_crtc->cmdq_client.chan) { + struct device_link *link; + + /* add devlink to cmdq dev to make sure suspend/resume order is correct */ + link = device_link_add(dev, mtk_crtc->cmdq_client.chan->mbox->dev, + DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS); + if (!link) { + dev_err(dev, "Unable to link dev=%s\n", + dev_name(mtk_crtc->cmdq_client.chan->mbox->dev)); + } + ret = of_property_read_u32_index(priv->mutex_node, "mediatek,gce-events", drm_crtc_index(_crtc->base), -- 2.18.0
[PATCH 1/3] mialbox: move cmdq suspend, resume and remove after cmdq_mbox_flush
Move the function order to make sure cmdq_mbox_flush is declared before cmdq_suspend calling it. Signed-off-by: jason-jh.lin --- drivers/mailbox/mtk-cmdq-mailbox.c | 84 +++--- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c index c591dab9d5a4..03f9ed4c5131 100644 --- a/drivers/mailbox/mtk-cmdq-mailbox.c +++ b/drivers/mailbox/mtk-cmdq-mailbox.c @@ -296,48 +296,6 @@ static irqreturn_t cmdq_irq_handler(int irq, void *dev) return IRQ_HANDLED; } -static int cmdq_suspend(struct device *dev) -{ - struct cmdq *cmdq = dev_get_drvdata(dev); - struct cmdq_thread *thread; - int i; - bool task_running = false; - - cmdq->suspended = true; - - for (i = 0; i < cmdq->thread_nr; i++) { - thread = >thread[i]; - if (!list_empty(>task_busy_list)) { - task_running = true; - break; - } - } - - if (task_running) - dev_warn(dev, "exist running task(s) in suspend\n"); - - clk_bulk_unprepare(cmdq->gce_num, cmdq->clocks); - - return 0; -} - -static int cmdq_resume(struct device *dev) -{ - struct cmdq *cmdq = dev_get_drvdata(dev); - - WARN_ON(clk_bulk_prepare(cmdq->gce_num, cmdq->clocks)); - cmdq->suspended = false; - return 0; -} - -static int cmdq_remove(struct platform_device *pdev) -{ - struct cmdq *cmdq = platform_get_drvdata(pdev); - - clk_bulk_unprepare(cmdq->gce_num, cmdq->clocks); - return 0; -} - static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data) { struct cmdq_pkt *pkt = (struct cmdq_pkt *)data; @@ -521,6 +479,48 @@ static struct mbox_chan *cmdq_xlate(struct mbox_controller *mbox, return >chans[ind]; } +static int cmdq_suspend(struct device *dev) +{ + struct cmdq *cmdq = dev_get_drvdata(dev); + struct cmdq_thread *thread; + int i; + bool task_running = false; + + cmdq->suspended = true; + + for (i = 0; i < cmdq->thread_nr; i++) { + thread = >thread[i]; + if (!list_empty(>task_busy_list)) { + task_running = true; + break; + } + } + + if (task_running) + dev_warn(dev, "exist running task(s) in suspend\n"); + + clk_bulk_unprepare(cmdq->gce_num, cmdq->clocks); + + return 0; +} + +static int cmdq_resume(struct device *dev) +{ + struct cmdq *cmdq = dev_get_drvdata(dev); + + WARN_ON(clk_bulk_prepare(cmdq->gce_num, cmdq->clocks)); + cmdq->suspended = false; + return 0; +} + +static int cmdq_remove(struct platform_device *pdev) +{ + struct cmdq *cmdq = platform_get_drvdata(pdev); + + clk_bulk_unprepare(cmdq->gce_num, cmdq->clocks); + return 0; +} + static int cmdq_probe(struct platform_device *pdev) { struct device *dev = >dev; -- 2.18.0
[PATCH] drm/panel-edp: modify Kconfig to prevent build error
When CONFIG_DRM_KMS_HELPER=m and CONFIG_DRM_PANEL_EDP=y, there is a build error in gpu/drm/panel/panel-edp.o: arm-linux-gnueabi-ld: drivers/gpu/drm/panel/panel-edp.o: in function `panel_edp_probe': panel-edp.c:(.text+0xf38): undefined reference to `drm_panel_dp_aux_backlight' Fix this by limiting DRM_PANEL_DEP by the value of the DRM_KMS_HELPER symbol. Fixes: 5f04e7ce392d ("drm/panel-edp: Split eDP panels out of panel-simple") Signed-off-by: Randy Dunlap Reported-by: kernel test robot Cc: Arnd Bergmann Cc: Daniel Vetter Cc: Javier Martinez Canillas Cc: Sam Ravnborg Cc: Douglas Anderson Cc: Linus Walleij Cc: Thierry Reding Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/panel/Kconfig |1 + 1 file changed, 1 insertion(+) --- linux-next-2027.orig/drivers/gpu/drm/panel/Kconfig +++ linux-next-2027/drivers/gpu/drm/panel/Kconfig @@ -104,6 +104,7 @@ config DRM_PANEL_EDP depends on OF depends on BACKLIGHT_CLASS_DEVICE depends on PM + depends on DRM_KMS_HELPER select VIDEOMODE_HELPERS select DRM_DP_AUX_BUS help
Re: [PATCH 2/2] drm/sched: fix dropping the last fence ref
Am 16.11.21 um 19:07 schrieb Rob Clark: On Tue, Nov 16, 2021 at 8:37 AM Daniel Vetter wrote: On Tue, Nov 16, 2021 at 10:25:19AM +0100, Christian König wrote: We need to grab another ref before trying to add the fence to the sched job and not after. Signed-off-by: Christian König Reviewed-by: Daniel Vetter I wondered first why this goes boom, but then I realized that in some cases add_dependency() drops the reference of the passed-in fence. Please also add the Fixes: line like in the previous patch. oh, I sent https://patchwork.freedesktop.org/patch/463329/ before I saw this.. it already has the fixes tag, and IMO a better description, so I think you can just pick that one instead Yeah, agree. You also have the missing Fixes line already. Going to add Daniels rb to your patch as well since it is technically the same. Thanks, Christian. BR, -R Cheers, Daniel --- drivers/gpu/drm/scheduler/sched_main.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 94fe51b3caa2..400d201c3c28 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -704,12 +704,14 @@ int drm_sched_job_add_implicit_dependencies(struct drm_sched_job *job, int ret; dma_resv_for_each_fence(, obj->resv, write, fence) { - ret = drm_sched_job_add_dependency(job, fence); - if (ret) - return ret; - /* Make sure to grab an additional ref on the added fence */ dma_fence_get(fence); + + ret = drm_sched_job_add_dependency(job, fence); + if (ret) { + dma_fence_put(fence); + return ret; + } } return 0; } -- 2.25.1 -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v3 0/4] Add Aspeed AST2600 soc display support
On Wed, 17 Nov 2021 at 05:45, tommy-huang wrote: > > v3: > Refine the patch for clear separate purpose. > Skip to send devicetree patch Thanks Tommy. A few things: - Set up your authorship in git: git config --global user.name "Tommy Haung" - The "Add AST2600 chip support" patch is the same as the one I sent, you can put mine back in your series now - We should add a device tree bindings document Can you confirm you tested these changes on both the ast2500 and the ast2600? How did you test? Cheers, Joel > > v2: > Remove some unnecessary patch. > Refine for reviwer request. > > v1: > First add patch. > > Joel Stanley (2): > ARM: dts: aspeed: Add GFX node to AST2600 > ARM: dts: aspeed: ast2600-evb: Enable GFX device > > tommy-huang (2): > drm/aspeed: Update INTR_STS handling > drm/aspeed: Add AST2600 chip support > > arch/arm/boot/dts/aspeed-ast2600-evb.dts | 18 ++ > arch/arm/boot/dts/aspeed-g6.dtsi | 11 +++ > drivers/gpu/drm/aspeed/aspeed_gfx.h | 1 + > drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 15 ++- > 4 files changed, 44 insertions(+), 1 deletion(-) > > -- > 2.17.1 >
Re: [PATCH v7 19/20] drm/mediatek: modify mediatek-drm for mt8195 multi mmsys support
Hi Chun-Kuang, Thanks for the review. On Thu, 2021-11-11 at 07:44 +0800, Chun-Kuang Hu wrote: > Hi, Nancy: > > Nancy.Lin 於 2021年10月29日 週五 下午3:52寫道: > > > > MT8195 have two mmsys. Modify drm for MT8195 multi-mmsys support. > > The two mmsys (vdosys0 and vdosys1) will bring up two drm drivers, > > only one drm driver register as the drm device. > > Each drm driver binds its own component. The last bind drm driver > > allocates and registers the drm device to drm core. > > Each crtc path is created with the corresponding drm driver data. > > > > Signed-off-by: Nancy.Lin > > --- > > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 24 +- > > drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 3 +- > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 301 ++ > > -- > > drivers/gpu/drm/mediatek/mtk_drm_drv.h | 9 +- > > 4 files changed, 249 insertions(+), 88 deletions(-) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > index ea285795776f..25580106a2c4 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > @@ -846,21 +846,28 @@ static int > > mtk_drm_crtc_init_comp_planes(struct drm_device *drm_dev, > > } > > > > int mtk_drm_crtc_create(struct drm_device *drm_dev, > > - const enum mtk_ddp_comp_id *path, unsigned > > int path_len) > > + const enum mtk_ddp_comp_id *path, unsigned > > int path_len, > > + int priv_data_index) > > { > > struct mtk_drm_private *priv = drm_dev->dev_private; > > struct device *dev = drm_dev->dev; > > struct mtk_drm_crtc *mtk_crtc; > > unsigned int num_comp_planes = 0; > > - int pipe = priv->num_pipes; > > int ret; > > int i; > > bool has_ctm = false; > > uint gamma_lut_size = 0; > > + struct drm_crtc *tmp; > > + int crtc_i = 0; > > > > if (!path) > > return 0; > > > > + priv = priv->all_drm_private[priv_data_index]; > > + > > + drm_for_each_crtc(tmp, drm_dev) > > + crtc_i++; > > + > > for (i = 0; i < path_len; i++) { > > enum mtk_ddp_comp_id comp_id = path[i]; > > struct device_node *node; > > @@ -872,7 +879,7 @@ int mtk_drm_crtc_create(struct drm_device > > *drm_dev, > > if (!node) { > > dev_info(dev, > > "Not creating crtc %d because > > component %d is disabled or missing\n", > > -pipe, comp_id); > > +crtc_i, comp_id); > > return 0; > > } > > > > @@ -925,29 +932,28 @@ int mtk_drm_crtc_create(struct drm_device > > *drm_dev, > > > > for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) { > > ret = mtk_drm_crtc_init_comp_planes(drm_dev, > > mtk_crtc, i, > > - pipe); > > + crtc_i); > > if (ret) > > return ret; > > } > > > > - ret = mtk_drm_crtc_init(drm_dev, mtk_crtc, pipe); > > + ret = mtk_drm_crtc_init(drm_dev, mtk_crtc, crtc_i); > > if (ret < 0) > > return ret; > > > > if (gamma_lut_size) > > drm_mode_crtc_set_gamma_size(_crtc->base, > > gamma_lut_size); > > drm_crtc_enable_color_mgmt(_crtc->base, 0, has_ctm, > > gamma_lut_size); > > - priv->num_pipes++; > > mutex_init(_crtc->hw_lock); > > > > #if IS_REACHABLE(CONFIG_MTK_CMDQ) > > + i = (priv->data->mmsys_dev_num > 1) ? 0 : > > drm_crtc_index(_crtc->base); > > I think even though mmsys_dev_num > 1, each mmsys could have more > than one crtc. > OK, I will refine the logic. > > mtk_crtc->cmdq_client.client.dev = mtk_crtc->mmsys_dev; > > mtk_crtc->cmdq_client.client.tx_block = false; > > mtk_crtc->cmdq_client.client.knows_txdone = true; > > mtk_crtc->cmdq_client.client.rx_callback = ddp_cmdq_cb; > > mtk_crtc->cmdq_client.chan = > > - mbox_request_channel(_crtc- > > >cmdq_client.client, > > -drm_crtc_index(_cr > > tc->base)); > > + mbox_request_channel(_crtc- > > >cmdq_client.client, i); > > if (IS_ERR(mtk_crtc->cmdq_client.chan)) { > > dev_dbg(dev, "mtk_crtc %d failed to create mailbox > > client, writing register by CPU now\n", > > drm_crtc_index(_crtc->base)); > > @@ -957,7 +963,7 @@ int mtk_drm_crtc_create(struct drm_device > > *drm_dev, > > if (mtk_crtc->cmdq_client.chan) { > > ret = of_property_read_u32_index(priv->mutex_node, > > "mediatek,gce- > > events", > > -
[PATCH v3 1/4] ARM: dts: aspeed: Add GFX node to AST2600
From: Joel Stanley The GFX device is present in the AST2600 SoC. Signed-off-by: Joel Stanley Signed-off-by: tommy-huang --- arch/arm/boot/dts/aspeed-g6.dtsi | 11 +++ 1 file changed, 11 insertions(+) diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi index 1b47be1704f8..e38c3742761b 100644 --- a/arch/arm/boot/dts/aspeed-g6.dtsi +++ b/arch/arm/boot/dts/aspeed-g6.dtsi @@ -351,6 +351,17 @@ quality = <100>; }; + gfx: display@1e6e6000 { + compatible = "aspeed,ast2600-gfx", "aspeed,ast2500-gfx", "syscon"; + reg = <0x1e6e6000 0x1000>; + reg-io-width = <4>; + clocks = < ASPEED_CLK_GATE_D1CLK>; + resets = < ASPEED_RESET_GRAPHICS>; + syscon = <>; + status = "disabled"; + interrupts = ; + }; + xdma: xdma@1e6e7000 { compatible = "aspeed,ast2600-xdma"; reg = <0x1e6e7000 0x100>; -- 2.17.1
[PATCH v3 4/4] drm/aspeed: Add AST2600 chip support
Add AST2600 chip support and setting. Signed-off-by: tommy-huang --- drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c index d4b56b3c7597..d10246b1d1c2 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c @@ -82,9 +82,18 @@ static const struct aspeed_gfx_config ast2500_config = { .scan_line_max = 128, }; +static const struct aspeed_gfx_config ast2600_config = { + .dac_reg = 0xc0, + .int_clear_reg = 0x68, + .vga_scratch_reg = 0x50, + .throd_val = CRT_THROD_LOW(0x50) | CRT_THROD_HIGH(0x70), + .scan_line_max = 128, +}; + static const struct of_device_id aspeed_gfx_match[] = { { .compatible = "aspeed,ast2400-gfx", .data = _config }, { .compatible = "aspeed,ast2500-gfx", .data = _config }, + { .compatible = "aspeed,ast2600-gfx", .data = _config }, { }, }; MODULE_DEVICE_TABLE(of, aspeed_gfx_match); -- 2.17.1
[PATCH v3 3/4] drm/aspeed: Update INTR_STS handling
Add interrupt clear register define for further chip support. Signed-off-by: tommy-huang --- drivers/gpu/drm/aspeed/aspeed_gfx.h | 1 + drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 6 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h index 96501152bafa..4e6a442c3886 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx.h +++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h @@ -12,6 +12,7 @@ struct aspeed_gfx { struct regmap *scu; u32 dac_reg; + u32 int_clr_reg; u32 vga_scratch_reg; u32 throd_val; u32 scan_line_max; diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c index b53fee6f1c17..d4b56b3c7597 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c @@ -60,6 +60,7 @@ struct aspeed_gfx_config { u32 dac_reg;/* DAC register in SCU */ + u32 int_clear_reg; /* Interrupt clear register */ u32 vga_scratch_reg;/* VGA scratch register in SCU */ u32 throd_val; /* Default Threshold Seting */ u32 scan_line_max; /* Max memory size of one scan line */ @@ -67,6 +68,7 @@ struct aspeed_gfx_config { static const struct aspeed_gfx_config ast2400_config = { .dac_reg = 0x2c, + .int_clear_reg = 0x60, .vga_scratch_reg = 0x50, .throd_val = CRT_THROD_LOW(0x1e) | CRT_THROD_HIGH(0x12), .scan_line_max = 64, @@ -74,6 +76,7 @@ static const struct aspeed_gfx_config ast2400_config = { static const struct aspeed_gfx_config ast2500_config = { .dac_reg = 0x2c, + .int_clear_reg = 0x60, .vga_scratch_reg = 0x50, .throd_val = CRT_THROD_LOW(0x24) | CRT_THROD_HIGH(0x3c), .scan_line_max = 128, @@ -119,7 +122,7 @@ static irqreturn_t aspeed_gfx_irq_handler(int irq, void *data) if (reg & CRT_CTRL_VERTICAL_INTR_STS) { drm_crtc_handle_vblank(>pipe.crtc); - writel(reg, priv->base + CRT_CTRL1); + writel(reg, priv->base + priv->int_clr_reg); return IRQ_HANDLED; } @@ -147,6 +150,7 @@ static int aspeed_gfx_load(struct drm_device *drm) config = match->data; priv->dac_reg = config->dac_reg; + priv->int_clr_reg = config->int_clear_reg; priv->vga_scratch_reg = config->vga_scratch_reg; priv->throd_val = config->throd_val; priv->scan_line_max = config->scan_line_max; -- 2.17.1
[PATCH v3 0/4] Add Aspeed AST2600 soc display support
v3: Refine the patch for clear separate purpose. Skip to send devicetree patch v2: Remove some unnecessary patch. Refine for reviwer request. v1: First add patch. Joel Stanley (2): ARM: dts: aspeed: Add GFX node to AST2600 ARM: dts: aspeed: ast2600-evb: Enable GFX device tommy-huang (2): drm/aspeed: Update INTR_STS handling drm/aspeed: Add AST2600 chip support arch/arm/boot/dts/aspeed-ast2600-evb.dts | 18 ++ arch/arm/boot/dts/aspeed-g6.dtsi | 11 +++ drivers/gpu/drm/aspeed/aspeed_gfx.h | 1 + drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 15 ++- 4 files changed, 44 insertions(+), 1 deletion(-) -- 2.17.1
[PATCH v3 2/4] ARM: dts: aspeed: ast2600-evb: Enable GFX device
From: Joel Stanley Enable the GFX device with a framebuffer memory region. Signed-off-by: Joel Stanley Signed-off-by: tommy-huang --- arch/arm/boot/dts/aspeed-ast2600-evb.dts | 18 ++ 1 file changed, 18 insertions(+) diff --git a/arch/arm/boot/dts/aspeed-ast2600-evb.dts b/arch/arm/boot/dts/aspeed-ast2600-evb.dts index b7eb552640cb..e223dad2abd0 100644 --- a/arch/arm/boot/dts/aspeed-ast2600-evb.dts +++ b/arch/arm/boot/dts/aspeed-ast2600-evb.dts @@ -23,6 +23,19 @@ reg = <0x8000 0x8000>; }; + reserved-memory { + #address-cells = <1>; + #size-cells = <1>; + ranges; + + gfx_memory: framebuffer { + size = <0x0100>; + alignment = <0x0100>; + compatible = "shared-dma-pool"; + reusable; + }; + }; + vcc_sdhci0: regulator-vcc-sdhci0 { compatible = "regulator-fixed"; regulator-name = "SDHCI0 Vcc"; @@ -300,3 +313,8 @@ vqmmc-supply = <_sdhci1>; clk-phase-sd-hs = <7>, <200>; }; + + { + status = "okay"; + memory-region = <_memory>; +}; -- 2.17.1
Re: [PATCH] lontium-lt9611: check a different register bit for HDMI sensing
Hi John, On 16-11-21, 20:47, John Stultz wrote: > On Tue, Nov 16, 2021 at 6:07 PM Peter Collingbourne wrote: > > > > It has been observed that with certain monitors such as the HP Z27n, > > the register 0x825e reads a value of 0x79 when the HDMI cable is > > connected and 0x78 when it is disconnected, i.e. bit 0 appears > > to correspond to the HDMI connection status and bit 2 is never > > set. Therefore, change the driver to check bit 0 instead of bit 2. > > > > Signed-off-by: Peter Collingbourne > > Link: > > https://linux-review.googlesource.com/id/I7e76411127e1ce4988a3f6d0c8ba5f1c3d880c23 > > --- > > N.B. I don't currently have easy access to a monitor that works > > with the existing driver, so it would be great if people with > > monitors that currently work could test this patch to make sure > > that it doesn't introduce any regressions. Otherwise I will change > > it to check both bits. > > So very interesting! I gave this a spin with my monitors and it works fine. > > Vinod: Can you check the datasheet to see if the wrong bit is being used here? Well the document I have does not list this register :( I have asked Lontium folks about this register description, will update if I hear back. Meanwhile we can get this patch tested with different monitors. Thanks -- ~Vinod
Re: [PATCH] lontium-lt9611: check a different register bit for HDMI sensing
On 16-11-21, 22:49, Anibal Limon wrote: > Dmitry, > > May be this is the reason of my HP monitor not working in RB5. Rb5 has Lt9611UXC > > Regards, > Anibal > > El mar., 16 de noviembre de 2021 20:07, Peter Collingbourne > escribió: > > > It has been observed that with certain monitors such as the HP Z27n, > > the register 0x825e reads a value of 0x79 when the HDMI cable is > > connected and 0x78 when it is disconnected, i.e. bit 0 appears > > to correspond to the HDMI connection status and bit 2 is never > > set. Therefore, change the driver to check bit 0 instead of bit 2. > > > > Signed-off-by: Peter Collingbourne > > Link: > > https://linux-review.googlesource.com/id/I7e76411127e1ce4988a3f6d0c8ba5f1c3d880c23 > > --- > > N.B. I don't currently have easy access to a monitor that works > > with the existing driver, so it would be great if people with > > monitors that currently work could test this patch to make sure > > that it doesn't introduce any regressions. Otherwise I will change > > it to check both bits. > > > > drivers/gpu/drm/bridge/lontium-lt9611.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c > > b/drivers/gpu/drm/bridge/lontium-lt9611.c > > index 29b1ce2140ab..71f1db802916 100644 > > --- a/drivers/gpu/drm/bridge/lontium-lt9611.c > > +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c > > @@ -586,7 +586,7 @@ lt9611_connector_detect(struct drm_connector > > *connector, bool force) > > int connected = 0; > > > > regmap_read(lt9611->regmap, 0x825e, _val); > > - connected = (reg_val & BIT(2)); > > + connected = (reg_val & BIT(0)); > > > > lt9611->status = connected ? connector_status_connected : > > connector_status_disconnected; > > @@ -926,7 +926,7 @@ static enum drm_connector_status > > lt9611_bridge_detect(struct drm_bridge *bridge) > > int connected; > > > > regmap_read(lt9611->regmap, 0x825e, _val); > > - connected = reg_val & BIT(2); > > + connected = reg_val & BIT(0); > > > > lt9611->status = connected ? connector_status_connected : > > connector_status_disconnected; > > -- > > 2.34.0.rc1.387.gb447b232ab-goog > > > > -- ~Vinod
Re: [PATCH] lontium-lt9611: check a different register bit for HDMI sensing
On Tue, Nov 16, 2021 at 6:07 PM Peter Collingbourne wrote: > > It has been observed that with certain monitors such as the HP Z27n, > the register 0x825e reads a value of 0x79 when the HDMI cable is > connected and 0x78 when it is disconnected, i.e. bit 0 appears > to correspond to the HDMI connection status and bit 2 is never > set. Therefore, change the driver to check bit 0 instead of bit 2. > > Signed-off-by: Peter Collingbourne > Link: > https://linux-review.googlesource.com/id/I7e76411127e1ce4988a3f6d0c8ba5f1c3d880c23 > --- > N.B. I don't currently have easy access to a monitor that works > with the existing driver, so it would be great if people with > monitors that currently work could test this patch to make sure > that it doesn't introduce any regressions. Otherwise I will change > it to check both bits. So very interesting! I gave this a spin with my monitors and it works fine. Vinod: Can you check the datasheet to see if the wrong bit is being used here? thanks -john
Re: [PATCH v7 18/20] drm/mediatek: add ovl_adaptor support for MT8195
Hi Chun-Kuang, Thanks for the review. On Fri, 2021-11-05 at 08:29 +0800, Chun-Kuang Hu wrote: > Hi, Nancy: > > Nancy.Lin 於 2021年10月29日 週五 下午3:52寫道: > > > > Add ovl_adaptor driver for MT8195. > > Ovl_adaptor is an encapsulated module and designed for simplified > > DRM control flow. This module is composed of 8 RDMAs, 4 MERGEs and > > an ETHDR. Two RDMAs merge into one layer, so this module support 4 > > layers. > > > > Signed-off-by: Nancy.Lin > > --- > > drivers/gpu/drm/mediatek/Makefile | 1 + > > drivers/gpu/drm/mediatek/mtk_disp_drv.h | 16 + > > .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 436 > > ++ > > drivers/gpu/drm/mediatek/mtk_drm_drv.h| 1 + > > 4 files changed, 454 insertions(+) > > create mode 100644 drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > > > > diff --git a/drivers/gpu/drm/mediatek/Makefile > > b/drivers/gpu/drm/mediatek/Makefile > > index fb158a1e7f06..3abd27d7c91d 100644 > > --- a/drivers/gpu/drm/mediatek/Makefile > > +++ b/drivers/gpu/drm/mediatek/Makefile > > @@ -6,6 +6,7 @@ mediatek-drm-y := mtk_disp_aal.o \ > > mtk_disp_gamma.o \ > > mtk_disp_merge.o \ > > mtk_disp_ovl.o \ > > + mtk_disp_ovl_adaptor.o \ > > mtk_disp_rdma.o \ > > mtk_drm_crtc.o \ > > mtk_drm_ddp_comp.o \ > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h > > b/drivers/gpu/drm/mediatek/mtk_disp_drv.h > > index 224a710bb537..cd9827402626 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h > > @@ -112,6 +112,22 @@ void mtk_rdma_enable_vblank(struct device > > *dev, > > void *vblank_cb_data); > > void mtk_rdma_disable_vblank(struct device *dev); > > > > +int mtk_ovl_adaptor_clk_enable(struct device *dev); > > +void mtk_ovl_adaptor_clk_disable(struct device *dev); > > +void mtk_ovl_adaptor_config(struct device *dev, unsigned int w, > > + unsigned int h, unsigned int vrefresh, > > + unsigned int bpc, struct cmdq_pkt > > *cmdq_pkt); > > +void mtk_ovl_adaptor_layer_config(struct device *dev, unsigned int > > idx, > > + struct mtk_plane_state *state, > > + struct cmdq_pkt *cmdq_pkt); > > +void mtk_ovl_adaptor_enable_vblank(struct device *dev, > > + void (*vblank_cb)(void *), > > + void *vblank_cb_data); > > +void mtk_ovl_adaptor_disable_vblank(struct device *dev); > > +void mtk_ovl_adaptor_start(struct device *dev); > > +void mtk_ovl_adaptor_stop(struct device *dev); > > +unsigned int mtk_ovl_adaptor_layer_nr(struct device *dev); > > + > > int mtk_mdp_rdma_clk_enable(struct device *dev); > > void mtk_mdp_rdma_clk_disable(struct device *dev); > > void mtk_mdp_rdma_start(struct device *dev, struct cmdq_pkt > > *cmdq_pkt); > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > > b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > > new file mode 100644 > > index ..148322597fa8 > > --- /dev/null > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c > > @@ -0,0 +1,436 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (c) 2021 MediaTek Inc. > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include "mtk_disp_drv.h" > > +#include "mtk_drm_crtc.h" > > +#include "mtk_drm_ddp_comp.h" > > +#include "mtk_drm_drv.h" > > +#include "mtk_ethdr.h" > > + > > +#define MTK_OVL_ADAPTOR_RDMA_MAX_WIDTH 1920 > > +#define MTK_OVL_ADAPTOR_LAYER_NUM 4 > > + > > +enum mtk_ovl_adaptor_comp_type { > > + OVL_ADAPTOR_TYPE_RDMA = 0, > > + OVL_ADAPTOR_TYPE_MERGE, > > + OVL_ADAPTOR_TYPE_ETHDR, > > + OVL_ADAPTOR_TYPE_NUM, > > +}; > > + > > +enum mtk_ovl_adaptor_comp_id { > > + OVL_ADAPTOR_MDP_RDMA0, > > + OVL_ADAPTOR_MDP_RDMA1, > > + OVL_ADAPTOR_MDP_RDMA2, > > + OVL_ADAPTOR_MDP_RDMA3, > > + OVL_ADAPTOR_MDP_RDMA4, > > + OVL_ADAPTOR_MDP_RDMA5, > > + OVL_ADAPTOR_MDP_RDMA6, > > + OVL_ADAPTOR_MDP_RDMA7, > > + OVL_ADAPTOR_MERGE0, > > + OVL_ADAPTOR_MERGE1, > > + OVL_ADAPTOR_MERGE2, > > + OVL_ADAPTOR_MERGE3, > > + OVL_ADAPTOR_ETHDR0, > > + OVL_ADAPTOR_ID_MAX > > +}; > > + > > +struct ovl_adaptor_comp_match { > > + enum mtk_ovl_adaptor_comp_type type; > > + int alias_id; > > +}; > > + > > +struct mtk_disp_ovl_adaptor { > > + struct device *ovl_adaptor_comp[OVL_ADAPTOR_ID_MAX]; > > + struct device *mmsys_dev; > > +}; > > + > > +static const char * const private_comp_stem[OVL_ADAPTOR_TYPE_NUM] > > = { > > + [OVL_ADAPTOR_TYPE_RDMA] = "vdo1_rdma", > > +
Re: [PATCH v7 16/20] drm/mediatek: add ETHDR support for MT8195
Hi Chun-Kuang, Thanks for the review. On Fri, 2021-11-05 at 07:59 +0800, Chun-Kuang Hu wrote: > Hi, Nancy: > > Nancy.Lin 於 2021年10月29日 週五 下午3:52寫道: > > > > ETHDR is a part of ovl_adaptor. > > ETHDR is designed for HDR video and graphics conversion in the > > external > > display path. It handles multiple HDR input types and performs tone > > mapping, color space/color format conversion, and then combine > > different layers, output the required HDR or SDR signal to the > > subsequent display path. > > > > Signed-off-by: Nancy.Lin > > --- > > drivers/gpu/drm/mediatek/Makefile| 1 + > > drivers/gpu/drm/mediatek/mtk_ethdr.c | 399 > > +++ > > drivers/gpu/drm/mediatek/mtk_ethdr.h | 25 ++ > > 3 files changed, 425 insertions(+) > > create mode 100644 drivers/gpu/drm/mediatek/mtk_ethdr.c > > create mode 100644 drivers/gpu/drm/mediatek/mtk_ethdr.h > > > > diff --git a/drivers/gpu/drm/mediatek/Makefile > > b/drivers/gpu/drm/mediatek/Makefile > > index 6e604a933ed0..fb158a1e7f06 100644 > > --- a/drivers/gpu/drm/mediatek/Makefile > > +++ b/drivers/gpu/drm/mediatek/Makefile > > @@ -14,6 +14,7 @@ mediatek-drm-y := mtk_disp_aal.o \ > > mtk_drm_plane.o \ > > mtk_dsi.o \ > > mtk_dpi.o \ > > + mtk_ethdr.o \ > > mtk_mdp_rdma.o > > > > obj-$(CONFIG_DRM_MEDIATEK) += mediatek-drm.o > > diff --git a/drivers/gpu/drm/mediatek/mtk_ethdr.c > > b/drivers/gpu/drm/mediatek/mtk_ethdr.c > > new file mode 100644 > > index ..86b584e9cf2c > > --- /dev/null > > +++ b/drivers/gpu/drm/mediatek/mtk_ethdr.c > > @@ -0,0 +1,399 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (c) 2021 MediaTek Inc. > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include "mtk_drm_crtc.h" > > +#include "mtk_drm_ddp_comp.h" > > +#include "mtk_drm_drv.h" > > +#include "mtk_ethdr.h" > > + > > +#define MIX_INTEN 0x4 > > +#define MIX_FME_CPL_INTEN BIT(1) > > +#define MIX_INTSTA 0x8 > > +#define MIX_EN 0xc > > +#define MIX_RST0x14 > > +#define MIX_ROI_SIZE 0x18 > > +#define MIX_DATAPATH_CON 0x1c > > +#define OUTPUT_NO_RND BIT(3) > > +#define SOURCE_RGB_SEL BIT(7) > > +#define BACKGROUND_RELAY (4 << 9) > > +#define MIX_ROI_BGCLR 0x20 > > +#define BGCLR_BLACK0xff00 > > +#define MIX_SRC_CON0x24 > > +#define MIX_SRC_L0_EN BIT(0) > > +#define MIX_L_SRC_CON(n) (0x28 + 0x18 * (n)) > > +#define NON_PREMULTI_SOURCE(2 << 12) > > +#define MIX_L_SRC_SIZE(n) (0x30 + 0x18 * (n)) > > +#define MIX_L_SRC_OFFSET(n)(0x34 + 0x18 * (n)) > > +#define MIX_FUNC_DCM0 0x120 > > +#define MIX_FUNC_DCM1 0x124 > > +#define MIX_FUNC_DCM_ENABLE0x > > + > > +#define HDR_VDO_FE_0804_HDR_DM_FE 0x804 > > +#define HDR_VDO_FE_0804_BYPASS_ALL 0xfd > > +#define HDR_GFX_FE_0204_GFX_HDR_FE 0x204 > > +#define HDR_GFX_FE_0204_BYPASS_ALL 0xfd > > +#define HDR_VDO_BE_0204_VDO_DM_BE 0x204 > > +#define HDR_VDO_BE_0204_BYPASS_ALL 0x7e > > + > > +#define MIXER_INX_MODE_BYPASS 0 > > +#define MIXER_INX_MODE_EVEN_EXTEND 1 > > +#define DEFAULT_9BIT_ALPHA 0x100 > > +#defineMIXER_ALPHA_AEN BIT(8) > > +#defineMIXER_ALPHA 0xff > > +#define ETHDR_CLK_NUM 13 > > + > > +enum mtk_ethdr_comp_id { > > + ETHDR_MIXER, > > + ETHDR_VDO_FE0, > > + ETHDR_VDO_FE1, > > + ETHDR_GFX_FE0, > > + ETHDR_GFX_FE1, > > + ETHDR_VDO_BE, > > + ETHDR_ADL_DS, > > + ETHDR_ID_MAX > > +}; > > + > > +struct mtk_ethdr_comp { > > + struct device *dev; > > + void __iomem*regs; > > + struct cmdq_client_reg cmdq_base; > > +}; > > + > > +struct mtk_ethdr { > > + struct mtk_ethdr_comp ethdr_comp[ETHDR_ID_MAX]; > > + struct clk_bulk_dataethdr_clk[ETHDR_CLK_NUM]; > > + struct device *mmsys_dev; > > + spinlock_t lock; /* protects vblank_cb and > > vblank_cb_data */ > > + void(*vblank_cb)(void *data); > > + void*vblank_cb_data; > > + int irq; > > +}; > > + > > +static const char * const ethdr_comp_str[] = { > > + "ETHDR_MIXER", > > + "ETHDR_VDO_FE0", > > + "ETHDR_VDO_FE1", > > + "ETHDR_GFX_FE0", > > + "ETHDR_GFX_FE1", > > +
Re: [PATCH 12/13] sysctl: add helper to register empty subdir
On Fri, May 29, 2020 at 08:03:02AM -0500, Eric W. Biederman wrote: > Luis Chamberlain writes: > > > The way to create a subdirectory from the base set of directories > > is a bit obscure, so provide a helper which makes this clear, and > > also helps remove boiler plate code required to do this work. > > I agreee calling: > register_sysctl("fs/binfmt_misc", sysctl_mount_point) > is a bit obscure but if you are going to make a wrapper > please make it the trivial one liner above. > > Say something that looks like: > struct sysctl_header *register_sysctl_mount_point(const char *path) > { > return register_sysctl(path, sysctl_mount_point); > } > > And yes please talk about a mount point and not an empty dir, as these > are permanently empty directories to serve as mount points. There are > some subtle but important permission checks this allows in the case of > unprivileged mounts. > > Further code like this belong in proc_sysctl.c next to all of the code > it is related to so that it is easier to see how to refactor the code if > necessary. Alrighty, it's been a while since this kernel/sysctl.c kitchen sink cleanup... so it's time to respin this now that the merge window is open. I already rebased patches, addressed all input and now just waiting to fix any compilation errors. I'm going to split the patches up into real small sets so to ensure we just get this through becauase getting this in otherwise is going to be hard. I'd appreciate folk's review once the patches start going out. I think a hard part will be deciding what tree this should got through. Luis
linux-next: build warning after merge of the drm-misc tree
Hi all, After merging the drm-misc tree, today's linux-next build (htmldocs) produced this warning: include/drm/gpu_scheduler.h:316: warning: Function parameter or member 'work' not described in 'drm_sched_job' Introduced by commit 542cff7893a3 ("drm/sched: Avoid lockdep spalt on killing a processes") -- Cheers, Stephen Rothwell pgpLzASo97yUw.pgp Description: OpenPGP digital signature
Re: [PATCH v3 11/12] drm/virtio: implement context init: add virtio_gpu_fence_event
On Tue, Nov 16, 2021 at 7:43 AM Daniel Vetter wrote: > On Mon, Nov 15, 2021 at 07:26:14PM +, Kasireddy, Vivek wrote: > > Hi Daniel, Greg, > > > > If it is the same or a similar crash reported here: > > > https://lists.freedesktop.org/archives/dri-devel/2021-November/330018.html > > and here: > https://lists.freedesktop.org/archives/dri-devel/2021-November/330212.html > > then the fix is already merged: > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d89c0c8322ecdc9a2ec84b959b6f766be082da76 Yeah but that still leaves the problem of why exaxtly virtgpu is > reinventing drm_poll here? > Can you please replace it with drm_poll like all other drivers, including > the ones that have private events? > Hi Daniel, Allow me to explain the use case a bit. It's for when virtgpu KMS is not used, but a special Wayland compositor does wayland passthrough instead: https://www.youtube.com/watch?v=WwrXqDERFm8https://www.youtube.com/watch?v=EkNBsBx501Q This technique has gained much popularity in the virtualized laptop space, where it offers better performance/user experience than virtgpu KMS. The relevant paravirtualized userspace is "Sommelier": https://chromium.googlesource.com/chromiumos/platform2/+/main/vm_tools/sommelier/ https://chromium.googlesource.com/chromiumos/platform2/+/main/vm_tools/sommelier/virtualization/virtgpu_channel.cc Previously, we were using the out-of-tree virtio-wl device and there were many discussions on how we could get this upstream: https://lists.freedesktop.org/archives/dri-devel/2017-December/160309.html https://lists.oasis-open.org/archives/virtio-dev/202002/msg5.html Extending virtgpu was deemed the least intrusive option: https://www.spinics.net/lists/kvm/msg159206.html We ultimately settled on the context type abstraction and used virtio_gpu_poll to tell the guest "hey, we have a Wayland event". The host response is actually in a buffer of type BLOB_MEM_GUEST. It is possible to use drm_poll(..), but that would have to be accompanied by a drm_read(..). You'll need to define a dummy VIRTGPU_EVENT_FENCE_SIGNALED in the uapi too. That's originally how I did it, but some pointed out that's unnecessary since the host response is in the BLOB_MEM_GUEST buffer and virtgpu event is a dummy event. So we decided just to modify virtio_gpu_poll(..) to have the desired semantics in that case. For the regular virtio-gpu KMS path, things remain unchanged. There are of course other ways to do it (perhaps polling a dma_fence), but that was the cleanest way we could find. It's not rare for virtio to "special things" (see virtio_dma_buf_ops, virtio_dma_ops), since they are in fake devices. We're open to other ideas, but hopefully that answers some of your questions. > Thanks, Daniel > > > > > Thanks, > > Vivek > > > > > On Sat, Nov 13, 2021 at 03:51:48PM +0100, Greg KH wrote: > > > > On Tue, Sep 21, 2021 at 04:20:23PM -0700, Gurchetan Singh wrote: > > > > > Similar to DRM_VMW_EVENT_FENCE_SIGNALED. Sends a pollable event > > > > > to the DRM file descriptor when a fence on a specific ring is > > > > > signaled. > > > > > > > > > > One difference is the event is not exposed via the UAPI -- this is > > > > > because host responses are on a shared memory buffer of type > > > > > BLOB_MEM_GUEST [this is the common way to receive responses with > > > > > virtgpu]. As such, there is no context specific read(..) > > > > > implementation either -- just a poll(..) implementation. > > > > > > > > > > Signed-off-by: Gurchetan Singh > > > > > Acked-by: Nicholas Verne > > > > > --- > > > > > drivers/gpu/drm/virtio/virtgpu_drv.c | 43 > +- > > > > > drivers/gpu/drm/virtio/virtgpu_drv.h | 7 + > > > > > drivers/gpu/drm/virtio/virtgpu_fence.c | 10 ++ > > > > > drivers/gpu/drm/virtio/virtgpu_ioctl.c | 34 > > > > > 4 files changed, 93 insertions(+), 1 deletion(-) > > > > > > > > This commit seems to cause a crash in a virtual drm gpu driver for > > > > Android. I have reverted this, and the next commit in the series > from > > > > Linus's tree and all is good again. > > > > > > > > Any ideas? > > > > > > Well no, but also this patch looks very questionable of hand-rolling > > > drm_poll. Yes you can do driver private events like > > > DRM_VMW_EVENT_FENCE_SIGNALED, that's fine. But you really should not > need > > > to hand-roll the poll callback. vmwgfx (which generally is a very old > > > driver which has lots of custom stuff, so not a great example) doesn't > do > > > that either. > > > > > > So that part should go no matter what I think. > > > -Daniel > > > -- > > > Daniel Vetter > > > Software Engineer, Intel Corporation > > > http://blog.ffwll.ch > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch >
Re: [PATCH 2/2] dt-bindings: panel: Introduce a panel-lvds binding
On Tue, 16 Nov 2021 15:35:03 +0100, Maxime Ripard wrote: > Following the previous patch, let's introduce a generic panel-lvds > binding that documents the panels that don't have any particular > constraint documented. > > Signed-off-by: Maxime Ripard > --- > .../bindings/display/panel/panel-lvds.yaml| 56 +++ > 1 file changed, 56 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/panel-lvds.yaml > 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/panel/panel-lvds.yaml:28:15: [warning] wrong indentation: expected 12 but found 14 (indentation) dtschema/dtc warnings/errors: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.example.dt.yaml: panel-lvds: compatible:0: 'advantech,idk-2121wr' is not one of ['auo,b101ew05', 'tbs,a711-panel'] From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.example.dt.yaml: panel-lvds: 'port' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/1555849 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.
Re: [PATCH v4 3/3] MAINTAINERS: Mark VMware mailing list entries as email aliases
> On Nov 16, 2021, at 17:41, Srivatsa S. Bhat wrote: > > From: Srivatsa S. Bhat (VMware) > > VMware mailing lists in the MAINTAINERS file are private lists meant > for VMware-internal review/notification for patches to the respective > subsystems. Anyone can post to these addresses, but there is no public > read access like open mailing lists, which makes them more like email > aliases instead (to reach out to reviewers). > > So update all the VMware mailing list references in the MAINTAINERS > file to mark them as such, using "R: email-al...@vmware.com". > > Signed-off-by: Srivatsa S. Bhat (VMware) > Cc: Zack Rusin > Cc: Nadav Amit > Cc: Vivek Thampi > Cc: Vishal Bhakta > Cc: Ronak Doshi > Cc: pv-driv...@vmware.com > Cc: linux-graphics-maintai...@vmware.com > Cc: dri-devel@lists.freedesktop.org > Cc: linux-r...@vger.kernel.org > Cc: linux-s...@vger.kernel.org > Cc: net...@vger.kernel.org > Cc: linux-in...@vger.kernel.org > Acked-by: Juergen Gross > --- > > MAINTAINERS | 22 +++--- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 01c7d1498c56..9b18fca73371 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6223,8 +6223,8 @@ T: git git://anongit.freedesktop.org/drm/drm-misc > F:drivers/gpu/drm/vboxvideo/ > > DRM DRIVER FOR VMWARE VIRTUAL GPU > -M: "VMware Graphics" > M:Zack Rusin > +R: VMware Graphics Reviewers > L:dri-devel@lists.freedesktop.org > S:Supported > T:git git://anongit.freedesktop.org/drm/drm-misc Acked-by: Zack Rusin z
Re: Build regressions/improvements in v5.16-rc1
On 11/16/21 5:59 PM, Nick Terrell wrote: On Nov 15, 2021, at 8:44 AM, Helge Deller wrote: On 11/15/21 17:12, Geert Uytterhoeven wrote: On Mon, Nov 15, 2021 at 4:54 PM Geert Uytterhoeven wrote: Below is the list of build error/warning regressions/improvements in v5.16-rc1[1] compared to v5.15[2]. Summarized: - build errors: +20/-13 - build warnings: +3/-28 Happy fixing! ;-) Thanks to the linux-next team for providing the build service. [1] http://kisskb.ellerman.id.au/kisskb/branch/linus/head/fa55b7dcdc43c1aa1ba12bca9d2dd4318c2a0dbf/ (all 90 configs) [2] http://kisskb.ellerman.id.au/kisskb/branch/linus/head/8bb7eca972ad531c9b149c0a51ab43a417385813/ (all 90 configs) *** ERRORS *** 20 error regressions: + /kisskb/src/arch/parisc/include/asm/jump_label.h: error: expected ':' before '__stringify': => 33:4, 18:4 + /kisskb/src/arch/parisc/include/asm/jump_label.h: error: label 'l_yes' defined but not used [-Werror=unused-label]: => 38:1, 23:1 due to static_branch_likely() in crypto/api.c parisc-allmodconfig fixed now in the parisc for-next git tree. + /kisskb/src/drivers/gpu/drm/msm/msm_drv.h: error: "COND" redefined [-Werror]: => 531 + /kisskb/src/lib/zstd/compress/zstd_double_fast.c: error: the frame size of 3252 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]: => 47:1 + /kisskb/src/lib/zstd/compress/zstd_double_fast.c: error: the frame size of 3360 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]: => 499:1 + /kisskb/src/lib/zstd/compress/zstd_double_fast.c: error: the frame size of 5344 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]: => 334:1 + /kisskb/src/lib/zstd/compress/zstd_double_fast.c: error: the frame size of 5380 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]: => 354:1 + /kisskb/src/lib/zstd/compress/zstd_fast.c: error: the frame size of 1824 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]: => 372:1 + /kisskb/src/lib/zstd/compress/zstd_fast.c: error: the frame size of 2224 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]: => 204:1 + /kisskb/src/lib/zstd/compress/zstd_fast.c: error: the frame size of 3800 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]: => 476:1 parisc-allmodconfig parisc needs much bigger frame sizes, so I'm not astonished here. During the v5.15 cycl I increased it to 1536 (from 1280), so I'm simply tempted to increase it this time to 4096, unless someone has a better idea This patch set should fix the zstd stack size warnings [0]. I’ve verified the fix using the same tooling: gcc-8-hppa-linux-gnu. I’ll send the PR to Linus tomorrow. I’ve been informed that it isn't strictly necessary to send the patches to the mailing list for bug fixes, but its already done, so I’ll wait and see if there is any feedback. IMO several (or many more) people would disagree with that. "strictly?" OK, it's probably possible that almost any patch could be merged without being on a mailing list, but it's not desirable (except in the case of "security" patches). -- ~Randy
[PATCH] drm/selftest: fix potential memleak in error branch
This patch try to fix the potential memleak in error branch. Signed-off-by: Bernard Zhao --- drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c index 6b4759ed6bfd..dbac073ed385 100644 --- a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c +++ b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c @@ -131,8 +131,10 @@ sideband_msg_req_encode_decode(struct drm_dp_sideband_msg_req_body *in) return false; txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL); - if (!txmsg) - return false; + if (!txmsg) { + result = false; + goto out; + } drm_dp_encode_sideband_req(in, txmsg); ret = drm_dp_decode_sideband_req(txmsg, out); -- 2.33.1
[PATCH] drm/aspeed: Fix vga_pw sysfs output
Before the drm driver had support for this file there was a driver that exposed the contents of the vga password register to userspace. It would present the entire register instead of interpreting it. The drm implementation chose to mask of the lower bit, without explaining why. This breaks the existing userspace, which is looking for 0xa8 in the lower byte. Change our implementation to expose the entire register. Fixes: 696029eb36c0 ("drm/aspeed: Add sysfs for output settings") Reported-by: Oskar Senft Signed-off-by: Joel Stanley --- drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c index b53fee6f1c17..65f172807a0d 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c @@ -291,7 +291,7 @@ vga_pw_show(struct device *dev, struct device_attribute *attr, char *buf) if (rc) return rc; - return sprintf(buf, "%u\n", reg & 1); + return sprintf(buf, "%u\n", reg); } static DEVICE_ATTR_RO(vga_pw); -- 2.33.0
Re: [PATCH v4 3/3] MAINTAINERS: Mark VMware mailing list entries as email aliases
On Tue, 2021-11-16 at 14:41 -0800, Srivatsa S. Bhat wrote: > From: Srivatsa S. Bhat (VMware) > > VMware mailing lists in the MAINTAINERS file are private lists meant > for VMware-internal review/notification for patches to the respective > subsystems. Anyone can post to these addresses, but there is no public > read access like open mailing lists, which makes them more like email > aliases instead (to reach out to reviewers). > > So update all the VMware mailing list references in the MAINTAINERS > file to mark them as such, using "R: email-al...@vmware.com". > > Signed-off-by: Srivatsa S. Bhat (VMware) Acked-by: Joe Perches > diff --git a/MAINTAINERS b/MAINTAINERS [] > @@ -6223,8 +6223,8 @@ T: git git://anongit.freedesktop.org/drm/drm-misc > F: drivers/gpu/drm/vboxvideo/ > > DRM DRIVER FOR VMWARE VIRTUAL GPU > -M: "VMware Graphics" > M: Zack Rusin > +R: VMware Graphics Reviewers etc...
[PATCH v4 3/3] MAINTAINERS: Mark VMware mailing list entries as email aliases
From: Srivatsa S. Bhat (VMware) VMware mailing lists in the MAINTAINERS file are private lists meant for VMware-internal review/notification for patches to the respective subsystems. Anyone can post to these addresses, but there is no public read access like open mailing lists, which makes them more like email aliases instead (to reach out to reviewers). So update all the VMware mailing list references in the MAINTAINERS file to mark them as such, using "R: email-al...@vmware.com". Signed-off-by: Srivatsa S. Bhat (VMware) Cc: Zack Rusin Cc: Nadav Amit Cc: Vivek Thampi Cc: Vishal Bhakta Cc: Ronak Doshi Cc: pv-driv...@vmware.com Cc: linux-graphics-maintai...@vmware.com Cc: dri-devel@lists.freedesktop.org Cc: linux-r...@vger.kernel.org Cc: linux-s...@vger.kernel.org Cc: net...@vger.kernel.org Cc: linux-in...@vger.kernel.org Acked-by: Juergen Gross --- MAINTAINERS | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 01c7d1498c56..9b18fca73371 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6223,8 +6223,8 @@ T:git git://anongit.freedesktop.org/drm/drm-misc F: drivers/gpu/drm/vboxvideo/ DRM DRIVER FOR VMWARE VIRTUAL GPU -M: "VMware Graphics" M: Zack Rusin +R: VMware Graphics Reviewers L: dri-devel@lists.freedesktop.org S: Supported T: git git://anongit.freedesktop.org/drm/drm-misc @@ -14410,7 +14410,7 @@ F: include/uapi/linux/ppdev.h PARAVIRT_OPS INTERFACE M: Juergen Gross M: Srivatsa S. Bhat (VMware) -M: "VMware, Inc." +R: VMware PV-Drivers Reviewers L: virtualizat...@lists.linux-foundation.org L: x...@kernel.org S: Supported @@ -20303,7 +20303,7 @@ F: tools/testing/vsock/ VMWARE BALLOON DRIVER M: Nadav Amit -M: "VMware, Inc." +R: VMware PV-Drivers Reviewers L: linux-ker...@vger.kernel.org S: Maintained F: drivers/misc/vmw_balloon.c @@ -20311,7 +20311,7 @@ F: drivers/misc/vmw_balloon.c VMWARE HYPERVISOR INTERFACE M: Srivatsa S. Bhat (VMware) M: Alexey Makhalov -M: "VMware, Inc." +R: VMware PV-Drivers Reviewers L: virtualizat...@lists.linux-foundation.org L: x...@kernel.org S: Supported @@ -20321,14 +20321,14 @@ F:arch/x86/kernel/cpu/vmware.c VMWARE PVRDMA DRIVER M: Adit Ranadive -M: VMware PV-Drivers +R: VMware PV-Drivers Reviewers L: linux-r...@vger.kernel.org S: Maintained F: drivers/infiniband/hw/vmw_pvrdma/ VMware PVSCSI driver M: Vishal Bhakta -M: VMware PV-Drivers +R: VMware PV-Drivers Reviewers L: linux-s...@vger.kernel.org S: Maintained F: drivers/scsi/vmw_pvscsi.c @@ -20336,7 +20336,7 @@ F: drivers/scsi/vmw_pvscsi.h VMWARE VIRTUAL PTP CLOCK DRIVER M: Vivek Thampi -M: "VMware, Inc." +R: VMware PV-Drivers Reviewers L: net...@vger.kernel.org S: Supported F: drivers/ptp/ptp_vmw.c @@ -20344,15 +20344,15 @@ F:drivers/ptp/ptp_vmw.c VMWARE VMCI DRIVER M: Jorgen Hansen M: Vishnu Dasa +R: VMware PV-Drivers Reviewers L: linux-ker...@vger.kernel.org -L: pv-driv...@vmware.com (private) S: Maintained F: drivers/misc/vmw_vmci/ VMWARE VMMOUSE SUBDRIVER M: Zack Rusin -M: "VMware Graphics" -M: "VMware, Inc." +R: VMware Graphics Reviewers +R: VMware PV-Drivers Reviewers L: linux-in...@vger.kernel.org S: Maintained F: drivers/input/mouse/vmmouse.c @@ -20360,7 +20360,7 @@ F: drivers/input/mouse/vmmouse.h VMWARE VMXNET3 ETHERNET DRIVER M: Ronak Doshi -M: pv-driv...@vmware.com +R: VMware PV-Drivers Reviewers L: net...@vger.kernel.org S: Maintained F: drivers/net/vmxnet3/
[PATCH v4 0/3] Update VMware maintainer entries
This series updates a few maintainer entries for VMware-maintained subsystems and cleans up references to VMware's private mailing lists to make it clear that they are effectively email-aliases to reach out to reviewers. Changes from v3->v4: - Remove Cc: sta...@vger.kernel.org from patches 1 and 2. Changes from v1->v3: - Add Zack as the named maintainer for vmmouse driver - Use R: to denote email-aliases for VMware reviewers Regards, Srivatsa --- Srivatsa S. Bhat (VMware) (3): MAINTAINERS: Update maintainers for paravirt ops and VMware hypervisor interface MAINTAINERS: Add Zack as maintainer of vmmouse driver MAINTAINERS: Mark VMware mailing list entries as email aliases MAINTAINERS | 30 +- 1 file changed, 17 insertions(+), 13 deletions(-)
linux-next: manual merge of the drm-misc tree with Linus' tree
Hi all, Today's linux-next merge of the drm-misc tree got a conflict in: drivers/gpu/drm/nouveau/dispnv50/disp.c between commit: d6c6a76f80a1 ("drm: Update MST First Link Slot Information Based on Encoding Format") from Linus' tree and commit: 606be062c2e5 ("drm/nouveau/kms/nv50-: Remove several set but not used variables "ret" in disp.c") from the drm-misc tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell diff --cc drivers/gpu/drm/nouveau/dispnv50/disp.c index 8e28403ea9b1,23fa9ecc2296.. --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@@ -1411,10 -1413,9 +1413,9 @@@ nv50_mstm_prepare(struct nv50_mstm *mst { struct nouveau_drm *drm = nouveau_drm(mstm->outp->base.base.dev); struct drm_encoder *encoder; - int ret; NV_ATOMIC(drm, "%s: mstm prepare\n", mstm->outp->base.base.name); - ret = drm_dp_update_payload_part1(>mgr, 1); - drm_dp_update_payload_part1(>mgr); ++ drm_dp_update_payload_part1(>mgr, 1); drm_for_each_encoder(encoder, mstm->outp->base.base.dev) { if (encoder->encoder_type == DRM_MODE_ENCODER_DPMST) { pgpUHI7Rx7rW0.pgp Description: OpenPGP digital signature
Re: [PATCH v4] drm/msm/dp: do not initialize phy until plugin interrupt received
On 11/15/2021 12:23 PM, khs...@codeaurora.org wrote: On 2021-11-09 13:38, Kuogee Hsieh wrote: From: Kuogee Hsieh Current DP drivers have regulators, clocks, irq and phy are grouped together within a function and executed not in a symmetric manner. This increase difficulty of code maintenance and limited code scalability. This patch divided the driver life cycle of operation into four states, resume (including booting up), dongle plugin, dongle unplugged and suspend. Regulators, core clocks and irq are grouped together and enabled at resume (or booting up) so that the DP controller is armed and ready to receive HPD plugin interrupts. HPD plugin interrupt is generated when a dongle plugs into DUT (device under test). Once HPD plugin interrupt is received, DP controller will initialize phy so that dpcd read/write will function and following link training can be proceeded successfully. DP phy will be disabled after main link is teared down at end of unplugged HPD interrupt handle triggered by dongle unplugged out of DUT. Finally regulators, code clocks and irq are disabled at corresponding suspension. Changes in V2: -- removed unnecessary dp_ctrl NULL check -- removed unnecessary phy init_count and power_count DRM_DEBUG_DP logs -- remove flip parameter out of dp_ctrl_irq_enable() -- add fixes tag Changes in V3: -- call dp_display_host_phy_init() instead of dp_ctrl_phy_init() at dp_display_host_init() for eDP Changes in V4: -- rewording commit text to match this commit changes Fixes: e91e3065a806 ("drm/msm/dp: Add DP compliance tests on Snapdragon Chipsets") Signed-off-by: Kuogee Hsieh --- any more comments? Hi Stephen/Bjorn, Any more comments? We like to merge this patch by the end of this month. Would you please give me your comments asap. Thanks, drivers/gpu/drm/msm/dp/dp_ctrl.c | 87 - drivers/gpu/drm/msm/dp/dp_ctrl.h | 9 ++-- drivers/gpu/drm/msm/dp/dp_display.c | 83 ++- 3 files changed, 105 insertions(+), 74 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c index 7ec155d..4788e8c 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c @@ -1364,60 +1364,54 @@ static int dp_ctrl_enable_stream_clocks(struct dp_ctrl_private *ctrl) return ret; } -int dp_ctrl_host_init(struct dp_ctrl *dp_ctrl, bool flip, bool reset) +void dp_ctrl_irq_enable(struct dp_ctrl *dp_ctrl) +{ + struct dp_ctrl_private *ctrl; + + ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); + + dp_catalog_ctrl_reset(ctrl->catalog); + + dp_catalog_ctrl_enable_irq(ctrl->catalog, true); +} + +void dp_ctrl_irq_disable(struct dp_ctrl *dp_ctrl) +{ + struct dp_ctrl_private *ctrl; + + ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); + + dp_catalog_ctrl_reset(ctrl->catalog); + + dp_catalog_ctrl_enable_irq(ctrl->catalog, false); +} + +void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl) { struct dp_ctrl_private *ctrl; struct dp_io *dp_io; struct phy *phy; - if (!dp_ctrl) { - DRM_ERROR("Invalid input data\n"); - return -EINVAL; - } - ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); dp_io = >parser->io; phy = dp_io->phy; - ctrl->dp_ctrl.orientation = flip; - - if (reset) - dp_catalog_ctrl_reset(ctrl->catalog); - - DRM_DEBUG_DP("flip=%d\n", flip); dp_catalog_ctrl_phy_reset(ctrl->catalog); phy_init(phy); - dp_catalog_ctrl_enable_irq(ctrl->catalog, true); - - return 0; } -/** - * dp_ctrl_host_deinit() - Uninitialize DP controller - * @dp_ctrl: Display Port Driver data - * - * Perform required steps to uninitialize DP controller - * and its resources. - */ -void dp_ctrl_host_deinit(struct dp_ctrl *dp_ctrl) +void dp_ctrl_phy_exit(struct dp_ctrl *dp_ctrl) { struct dp_ctrl_private *ctrl; struct dp_io *dp_io; struct phy *phy; - if (!dp_ctrl) { - DRM_ERROR("Invalid input data\n"); - return; - } - ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); dp_io = >parser->io; phy = dp_io->phy; - dp_catalog_ctrl_enable_irq(ctrl->catalog, false); + dp_catalog_ctrl_phy_reset(ctrl->catalog); phy_exit(phy); - - DRM_DEBUG_DP("Host deinitialized successfully\n"); } static bool dp_ctrl_use_fixed_nvid(struct dp_ctrl_private *ctrl) @@ -1895,8 +1889,14 @@ int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl) return ret; } + DRM_DEBUG_DP("Before, phy=%x init_count=%d power_on=%d\n", + (u32)(uintptr_t)phy, phy->init_count, phy->power_count); + phy_power_off(phy); + DRM_DEBUG_DP("After, phy=%x init_count=%d power_on=%d\n", + (u32)(uintptr_t)phy, phy->init_count, phy->power_count); + /* aux channel down, reinit phy */ phy_exit(phy); phy_init(phy); @@ -1905,23 +1905,6 @@ int dp_ctrl_off_link_stream(struct dp_ctrl
Re: [PATCH] In function nvkm_ioctl_map(), the variable "type" could be uninitialized if "nvkm_object_map()" returns error code, however, it does not check the return value and directly use the "type"
This is a very long patch name, it should probably be shorter and the details in the patch title moved into the actual commit description instead. Also a couple of things aren't formatted correctly: * Cc tag for stable is missing, see https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html * Fixes tag isn't formatted properly I generally recommend using `dim fixes` from https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html in order to get the correct stable kernel CC tag and Fixes: tag (you can drop any of the Ccs it gives you beyond the one to stable at vger dot kernel dot org. Also, if you could try to Cc: me on the next version - will help me respond faster :). On Mon, 2021-11-15 at 23:07 -0800, Yizhuo Zhai wrote: > Fixes:01326050391ce("drm/nouveau/core/object: allow arguments to > be passed to map function") > Signed-off-by: Yizhuo Zhai > --- > drivers/gpu/drm/nouveau/nvkm/core/ioctl.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c > b/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c > index 735cb6816f10..4264d9d79783 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c > +++ b/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c > @@ -266,6 +266,8 @@ nvkm_ioctl_map(struct nvkm_client *client, > ret = nvkm_object_map(object, data, size, , > >v0.handle, > >v0.length); > + if (ret) > + return ret; > if (type == NVKM_OBJECT_MAP_IO) > args->v0.type = NVIF_IOCTL_MAP_V0_IO; > else -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat
[PATCH v3 6/6] drm/amdgpu: add drm buddy support to amdgpu
- Remove drm_mm references and replace with drm buddy functionalities - Add res cursor support for drm buddy v2(Matthew Auld): - replace spinlock with mutex as we call kmem_cache_zalloc(..., GFP_KERNEL) in drm_buddy_alloc() function - lock drm_buddy_block_trim() function as it calls mark_free/mark_split are all globally visible Signed-off-by: Arunpravin --- .../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h| 97 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 6 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 263 ++ 3 files changed, 233 insertions(+), 133 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h index acfa207cf970..da12b4ff2e45 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h @@ -30,12 +30,15 @@ #include #include +#include "amdgpu_vram_mgr.h" + /* state back for walking over vram_mgr and gtt_mgr allocations */ struct amdgpu_res_cursor { uint64_tstart; uint64_tsize; uint64_tremaining; - struct drm_mm_node *node; + void*node; + uint32_tmem_type; }; /** @@ -52,27 +55,63 @@ static inline void amdgpu_res_first(struct ttm_resource *res, uint64_t start, uint64_t size, struct amdgpu_res_cursor *cur) { + struct drm_buddy_block *block; + struct list_head *head, *next; struct drm_mm_node *node; - if (!res || res->mem_type == TTM_PL_SYSTEM) { - cur->start = start; - cur->size = size; - cur->remaining = size; - cur->node = NULL; - WARN_ON(res && start + size > res->num_pages << PAGE_SHIFT); - return; - } + if (!res) + goto err_out; BUG_ON(start + size > res->num_pages << PAGE_SHIFT); - node = to_ttm_range_mgr_node(res)->mm_nodes; - while (start >= node->size << PAGE_SHIFT) - start -= node++->size << PAGE_SHIFT; + cur->mem_type = res->mem_type; + + switch (cur->mem_type) { + case TTM_PL_VRAM: + head = _amdgpu_vram_mgr_node(res)->blocks; + + block = list_first_entry_or_null(head, +struct drm_buddy_block, +link); + if (!block) + goto err_out; + + while (start >= amdgpu_node_size(block)) { + start -= amdgpu_node_size(block); + + next = block->link.next; + if (next != head) + block = list_entry(next, struct drm_buddy_block, link); + } + + cur->start = amdgpu_node_start(block) + start; + cur->size = min(amdgpu_node_size(block) - start, size); + cur->remaining = size; + cur->node = block; + break; + case TTM_PL_TT: + node = to_ttm_range_mgr_node(res)->mm_nodes; + while (start >= node->size << PAGE_SHIFT) + start -= node++->size << PAGE_SHIFT; + + cur->start = (node->start << PAGE_SHIFT) + start; + cur->size = min((node->size << PAGE_SHIFT) - start, size); + cur->remaining = size; + cur->node = node; + break; + default: + goto err_out; + } - cur->start = (node->start << PAGE_SHIFT) + start; - cur->size = min((node->size << PAGE_SHIFT) - start, size); + return; + +err_out: + cur->start = start; + cur->size = size; cur->remaining = size; - cur->node = node; + cur->node = NULL; + WARN_ON(res && start + size > res->num_pages << PAGE_SHIFT); + return; } /** @@ -85,7 +124,9 @@ static inline void amdgpu_res_first(struct ttm_resource *res, */ static inline void amdgpu_res_next(struct amdgpu_res_cursor *cur, uint64_t size) { - struct drm_mm_node *node = cur->node; + struct drm_buddy_block *block; + struct drm_mm_node *node; + struct list_head *next; BUG_ON(size > cur->remaining); @@ -99,9 +140,27 @@ static inline void amdgpu_res_next(struct amdgpu_res_cursor *cur, uint64_t size) return; } - cur->node = ++node; - cur->start = node->start << PAGE_SHIFT; - cur->size = min(node->size << PAGE_SHIFT, cur->remaining); + switch (cur->mem_type) { + case TTM_PL_VRAM: + block = cur->node; + + next = block->link.next; + block = list_entry(next, struct drm_buddy_block, link); + + cur->node = block; + cur->start =
[PATCH v3 5/6] drm/amdgpu: move vram inline functions into a header
Move shared vram inline functions and structs into a header file Signed-off-by: Arunpravin --- drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 51 1 file changed, 51 insertions(+) create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h new file mode 100644 index ..59983464cce5 --- /dev/null +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h @@ -0,0 +1,51 @@ +/* SPDX-License-Identifier: MIT + * Copyright 2021 Advanced Micro Devices, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + * + */ + +#ifndef __AMDGPU_VRAM_MGR_H__ +#define __AMDGPU_VRAM_MGR_H__ + +#include + +struct amdgpu_vram_mgr_node { + struct ttm_resource base; + struct list_head blocks; + unsigned long flags; +}; + +static inline u64 amdgpu_node_start(struct drm_buddy_block *block) +{ + return drm_buddy_block_offset(block); +} + +static inline u64 amdgpu_node_size(struct drm_buddy_block *block) +{ + return PAGE_SIZE << drm_buddy_block_order(block); +} + +static inline struct amdgpu_vram_mgr_node * +to_amdgpu_vram_mgr_node(struct ttm_resource *res) +{ + return container_of(res, struct amdgpu_vram_mgr_node, base); +} + +#endif -- 2.25.1
[PATCH v3 4/6] drm: implement a method to free unused pages
On contiguous allocation, we round up the size to the *next* power of 2, implement a function to free the unused pages after the newly allocate block. v2(Matthew Auld): - replace function name 'drm_buddy_free_unused_pages' with drm_buddy_block_trim - replace input argument name 'actual_size' with 'new_size' - add more validation checks for input arguments - add overlaps check to avoid needless searching and splitting - merged the below patch to see the feature in action - add free unused pages support to i915 driver - lock drm_buddy_block_trim() function as it calls mark_free/mark_split are all globally visible Signed-off-by: Arunpravin --- drivers/gpu/drm/drm_buddy.c | 108 ++ drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 10 ++ include/drm/drm_buddy.h | 4 + 3 files changed, 122 insertions(+) diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c index 0a9db2981188..943fe2ad27bf 100644 --- a/drivers/gpu/drm/drm_buddy.c +++ b/drivers/gpu/drm/drm_buddy.c @@ -284,6 +284,114 @@ static inline bool contains(u64 s1, u64 e1, u64 s2, u64 e2) return s1 <= s2 && e1 >= e2; } +/** + * drm_buddy_block_trim - free unused pages + * + * @mm: DRM buddy manager + * @new_size: original size requested + * @blocks: output list head to add allocated blocks + * + * For contiguous allocation, we round up the size to the nearest + * power of two value, drivers consume *actual* size, so remaining + * portions are unused and it can be freed. + * + * Returns: + * 0 on success, error code on failure. + */ +int drm_buddy_block_trim(struct drm_buddy_mm *mm, +u64 new_size, +struct list_head *blocks) +{ + struct drm_buddy_block *block; + struct drm_buddy_block *buddy; + u64 new_start; + u64 new_end; + LIST_HEAD(dfs); + u64 count = 0; + int err; + + if (!list_is_singular(blocks)) + return -EINVAL; + + block = list_first_entry(blocks, +struct drm_buddy_block, +link); + + if (!drm_buddy_block_is_allocated(block)) + return -EINVAL; + + if (new_size > drm_buddy_block_size(mm, block)) + return -EINVAL; + + if (!new_size && !IS_ALIGNED(new_size, mm->chunk_size)) + return -EINVAL; + + if (new_size == drm_buddy_block_size(mm, block)) + return 0; + + list_del(>link); + + new_start = drm_buddy_block_offset(block); + new_end = new_start + new_size - 1; + + mark_free(mm, block); + + list_add(>tmp_link, ); + + do { + u64 block_start; + u64 block_end; + + block = list_first_entry_or_null(, +struct drm_buddy_block, +tmp_link); + if (!block) + break; + + list_del(>tmp_link); + + if (count == new_size) + return 0; + + block_start = drm_buddy_block_offset(block); + block_end = block_start + drm_buddy_block_size(mm, block) - 1; + + if (!overlaps(new_start, new_end, block_start, block_end)) + continue; + + if (contains(new_start, new_end, block_start, block_end)) { + BUG_ON(!drm_buddy_block_is_free(block)); + + /* Allocate only required blocks */ + mark_allocated(block); + mm->avail -= drm_buddy_block_size(mm, block); + list_add_tail(>link, blocks); + count += drm_buddy_block_size(mm, block); + continue; + } + + if (!drm_buddy_block_is_split(block)) { + err = split_block(mm, block); + if (unlikely(err)) + goto err_undo; + } + + list_add(>right->tmp_link, ); + list_add(>left->tmp_link, ); + } while (1); + + return -ENOSPC; + +err_undo: + buddy = get_buddy(block); + if (buddy && + (drm_buddy_block_is_free(block) && +drm_buddy_block_is_free(buddy))) + __drm_buddy_free(mm, block); + return err; +} +EXPORT_SYMBOL(drm_buddy_block_trim); + static struct drm_buddy_block * alloc_range(struct drm_buddy_mm *mm, u64 start, u64 end, diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c index b6ed5b37cf18..5e1f8c443058 100644 --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c @@ -97,6 +97,16 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man, if (unlikely(err))
[PATCH v3 3/6] drm: implement top-down allocation method
Implemented a function which walk through the order list, compares the offset and returns the maximum offset block, this method is unpredictable in obtaining the high range address blocks which depends on allocation and deallocation. for instance, if driver requests address at a low specific range, allocator traverses from the root block and splits the larger blocks until it reaches the specific block and in the process of splitting, lower orders in the freelist are occupied with low range address blocks and for the subsequent TOPDOWN memory request we may return the low range blocks.To overcome this issue, we may go with the below approach. The other approach, sorting each order list entries in ascending order and compares the last entry of each order list in the freelist and return the max block. This creates sorting overhead on every drm_buddy_free() request and split up of larger blocks for a single page request. v2: - Fix alignment issues(Matthew Auld) - Remove unnecessary list_empty check(Matthew Auld) - merged the below patch to see the feature in action - add top-down alloc support to i915 driver Signed-off-by: Arunpravin --- drivers/gpu/drm/drm_buddy.c | 36 --- drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 3 ++ include/drm/drm_buddy.h | 1 + 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c index c9b18a29f8d1..0a9db2981188 100644 --- a/drivers/gpu/drm/drm_buddy.c +++ b/drivers/gpu/drm/drm_buddy.c @@ -361,6 +361,26 @@ alloc_range(struct drm_buddy_mm *mm, return ERR_PTR(err); } +static struct drm_buddy_block * +get_maxblock(struct list_head *head) +{ + struct drm_buddy_block *max_block = NULL, *node; + + max_block = list_first_entry_or_null(head, +struct drm_buddy_block, +link); + if (!max_block) + return NULL; + + list_for_each_entry(node, head, link) { + if (drm_buddy_block_offset(node) > + drm_buddy_block_offset(max_block)) + max_block = node; + } + + return max_block; +} + static struct drm_buddy_block * alloc_from_freelist(struct drm_buddy_mm *mm, unsigned int order, @@ -371,11 +391,17 @@ alloc_from_freelist(struct drm_buddy_mm *mm, int err; for (i = order; i <= mm->max_order; ++i) { - block = list_first_entry_or_null(>free_list[i], -struct drm_buddy_block, -link); - if (block) - break; + if (flags & DRM_BUDDY_TOPDOWN_ALLOCATION) { + block = get_maxblock(>free_list[i]); + if (block) + break; + } else { + block = list_first_entry_or_null(>free_list[i], +struct drm_buddy_block, +link); + if (block) + break; + } } if (!block) diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c index 9e3d130c3f42..b6ed5b37cf18 100644 --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c @@ -53,6 +53,9 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man, INIT_LIST_HEAD(_res->blocks); bman_res->mm = mm; + if (place->flags & TTM_PL_FLAG_TOPDOWN) + bman_res->flags |= DRM_BUDDY_TOPDOWN_ALLOCATION; + if (place->fpfn || lpfn != man->size) bman_res->flags |= DRM_BUDDY_RANGE_ALLOCATION; diff --git a/include/drm/drm_buddy.h b/include/drm/drm_buddy.h index 2ec3cbc9d5d7..cbe5e648440a 100644 --- a/include/drm/drm_buddy.h +++ b/include/drm/drm_buddy.h @@ -28,6 +28,7 @@ }) #define DRM_BUDDY_RANGE_ALLOCATION (1 << 0) +#define DRM_BUDDY_TOPDOWN_ALLOCATION (1 << 1) struct drm_buddy_block { #define DRM_BUDDY_HEADER_OFFSET GENMASK_ULL(63, 12) -- 2.25.1
[PATCH v3 2/6] drm: improve drm_buddy_alloc function
- Make drm_buddy_alloc a single function to handle range allocation and non-range allocation demands - Implemented a new function alloc_range() which allocates the requested power-of-two block comply with range limitations - Moved order computation and memory alignment logic from i915 driver to drm buddy v2: merged below changes to keep the build unbroken - drm_buddy_alloc_range() becomes obsolete and may be removed - enable ttm range allocation (fpfn / lpfn) support in i915 driver - apply enhanced drm_buddy_alloc() function to i915 driver v3(Matthew Auld): - Fix alignment issues and remove unnecessary list_empty check - add more validation checks for input arguments - make alloc_range() block allocations as bottom-up - optimize order computation logic - replace uint64_t with u64, which is preferred in the kernel Signed-off-by: Arunpravin --- drivers/gpu/drm/drm_buddy.c | 259 ++ drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 69 ++--- drivers/gpu/drm/i915/i915_ttm_buddy_manager.h | 2 + include/drm/drm_buddy.h | 22 +- 4 files changed, 203 insertions(+), 149 deletions(-) diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c index 39eb1d224bec..c9b18a29f8d1 100644 --- a/drivers/gpu/drm/drm_buddy.c +++ b/drivers/gpu/drm/drm_buddy.c @@ -274,63 +274,6 @@ void drm_buddy_free_list(struct drm_buddy_mm *mm, struct list_head *objects) } EXPORT_SYMBOL(drm_buddy_free_list); -/** - * drm_buddy_alloc - allocate power-of-two blocks - * - * @mm: DRM buddy manager to allocate from - * @order: size of the allocation - * - * The order value here translates to: - * - * 0 = 2^0 * mm->chunk_size - * 1 = 2^1 * mm->chunk_size - * 2 = 2^2 * mm->chunk_size - * - * Returns: - * allocated ptr to the _buddy_block on success - */ -struct drm_buddy_block * -drm_buddy_alloc(struct drm_buddy_mm *mm, unsigned int order) -{ - struct drm_buddy_block *block = NULL; - unsigned int i; - int err; - - for (i = order; i <= mm->max_order; ++i) { - block = list_first_entry_or_null(>free_list[i], -struct drm_buddy_block, -link); - if (block) - break; - } - - if (!block) - return ERR_PTR(-ENOSPC); - - BUG_ON(!drm_buddy_block_is_free(block)); - - while (i != order) { - err = split_block(mm, block); - if (unlikely(err)) - goto out_free; - - /* Go low */ - block = block->left; - i--; - } - - mark_allocated(block); - mm->avail -= drm_buddy_block_size(mm, block); - kmemleak_update_trace(block); - return block; - -out_free: - if (i != order) - __drm_buddy_free(mm, block); - return ERR_PTR(err); -} -EXPORT_SYMBOL(drm_buddy_alloc); - static inline bool overlaps(u64 s1, u64 e1, u64 s2, u64 e2) { return s1 <= e2 && e1 >= s2; @@ -341,52 +284,22 @@ static inline bool contains(u64 s1, u64 e1, u64 s2, u64 e2) return s1 <= s2 && e1 >= e2; } -/** - * drm_buddy_alloc_range - allocate range - * - * @mm: DRM buddy manager to allocate from - * @blocks: output list head to add allocated blocks - * @start: start of the allowed range for this block - * @size: size of the allocation - * - * Intended for pre-allocating portions of the address space, for example to - * reserve a block for the initial framebuffer or similar, hence the expectation - * here is that drm_buddy_alloc() is still the main vehicle for - * allocations, so if that's not the case then the drm_mm range allocator is - * probably a much better fit, and so you should probably go use that instead. - * - * Note that it's safe to chain together multiple alloc_ranges - * with the same blocks list - * - * Returns: - * 0 on success, error code on failure. - */ -int drm_buddy_alloc_range(struct drm_buddy_mm *mm, - struct list_head *blocks, - u64 start, u64 size) +static struct drm_buddy_block * +alloc_range(struct drm_buddy_mm *mm, + u64 start, u64 end, + unsigned int order) { struct drm_buddy_block *block; struct drm_buddy_block *buddy; - LIST_HEAD(allocated); LIST_HEAD(dfs); - u64 end; int err; int i; - if (size < mm->chunk_size) - return -EINVAL; - - if (!IS_ALIGNED(size | start, mm->chunk_size)) - return -EINVAL; - - if (range_overflows(start, size, mm->size)) - return -EINVAL; + end = end - 1; for (i = 0; i < mm->n_roots; ++i) list_add_tail(>roots[i]->tmp_link, ); - end = start + size - 1; - do { u64 block_start; u64 block_end; @@ -399,26 +312,26 @@ int
[PATCH v3 1/6] drm: move the buddy allocator from i915 into common drm
Move the base i915 buddy allocator code into drm - Move i915_buddy.h to include/drm - Move i915_buddy.c to drm root folder - Rename "i915" string with "drm" string wherever applicable - Rename "I915" string with "DRM" string wherever applicable - Fix header file dependencies - Fix alignment issues - add Makefile support for drm buddy - export functions and write kerneldoc description - Remove i915 selftest config check condition as buddy selftest will be moved to drm selftest folder cleanup i915 buddy references in i915 driver module and replace with drm buddy v2: - include header file in alphabetical order (Thomas) - merged changes listed in the body section into a single patch to keep the build intact (Christian, Jani) Signed-off-by: Arunpravin --- drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/drm_buddy.c | 521 ++ drivers/gpu/drm/drm_drv.c | 3 + drivers/gpu/drm/i915/Makefile | 1 - drivers/gpu/drm/i915/i915_buddy.c | 466 drivers/gpu/drm/i915/i915_buddy.h | 143 - drivers/gpu/drm/i915/i915_module.c| 3 - drivers/gpu/drm/i915/i915_scatterlist.c | 11 +- drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 33 +- drivers/gpu/drm/i915/i915_ttm_buddy_manager.h | 4 +- include/drm/drm_buddy.h | 153 + 11 files changed, 702 insertions(+), 638 deletions(-) create mode 100644 drivers/gpu/drm/drm_buddy.c delete mode 100644 drivers/gpu/drm/i915/i915_buddy.c delete mode 100644 drivers/gpu/drm/i915/i915_buddy.h create mode 100644 include/drm/drm_buddy.h diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 0dff40bb863c..dc61e91a3154 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -18,7 +18,7 @@ drm-y :=drm_aperture.o drm_auth.o drm_cache.o \ drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \ drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \ drm_client_modeset.o drm_atomic_uapi.o drm_hdcp.o \ - drm_managed.o drm_vblank_work.o + drm_managed.o drm_vblank_work.o drm_buddy.o drm-$(CONFIG_DRM_LEGACY) += drm_agpsupport.o drm_bufs.o drm_context.o drm_dma.o \ drm_legacy_misc.o drm_lock.o drm_memory.o drm_scatter.o \ diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c new file mode 100644 index ..39eb1d224bec --- /dev/null +++ b/drivers/gpu/drm/drm_buddy.c @@ -0,0 +1,521 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright © 2021 Intel Corporation + */ + +#include +#include + +#include + +static struct kmem_cache *slab_blocks; + +static struct drm_buddy_block *drm_block_alloc(struct drm_buddy_mm *mm, + struct drm_buddy_block *parent, + unsigned int order, + u64 offset) +{ + struct drm_buddy_block *block; + + BUG_ON(order > DRM_BUDDY_MAX_ORDER); + + block = kmem_cache_zalloc(slab_blocks, GFP_KERNEL); + if (!block) + return NULL; + + block->header = offset; + block->header |= order; + block->parent = parent; + + BUG_ON(block->header & DRM_BUDDY_HEADER_UNUSED); + return block; +} + +static void drm_block_free(struct drm_buddy_mm *mm, + struct drm_buddy_block *block) +{ + kmem_cache_free(slab_blocks, block); +} + +static void mark_allocated(struct drm_buddy_block *block) +{ + block->header &= ~DRM_BUDDY_HEADER_STATE; + block->header |= DRM_BUDDY_ALLOCATED; + + list_del(>link); +} + +static void mark_free(struct drm_buddy_mm *mm, + struct drm_buddy_block *block) +{ + block->header &= ~DRM_BUDDY_HEADER_STATE; + block->header |= DRM_BUDDY_FREE; + + list_add(>link, +>free_list[drm_buddy_block_order(block)]); +} + +static void mark_split(struct drm_buddy_block *block) +{ + block->header &= ~DRM_BUDDY_HEADER_STATE; + block->header |= DRM_BUDDY_SPLIT; + + list_del(>link); +} + +/** + * drm_buddy_init - init memory manager + * + * @mm: DRM buddy manager to initialize + * @size: size in bytes to manage + * @chunk_size: minimum page size in bytes for our allocations + * + * Initializes the memory manager and its resources. + * + * Returns: + * 0 on success, error code on failure. + */ +int drm_buddy_init(struct drm_buddy_mm *mm, u64 size, u64 chunk_size) +{ + unsigned int i; + u64 offset; + + if (size < chunk_size) + return -EINVAL; + + if (chunk_size < PAGE_SIZE) + return -EINVAL; + + if (!is_power_of_2(chunk_size)) + return -EINVAL; + + size = round_down(size, chunk_size); + + mm->size = size; + mm->avail = size; +
[PATCH] drm/i915: Check return value of i915_request_mark_eio before calling put
i915_request_mark_eio can return NULL and in some cases this return value was unconditionally passed to i915_request_put. i915_request_put in turn calls dma_fence_put on >fence. dma_fence_put checks for NULL and short circuits. This all just happens to work because rq->fence is first member in the i915_request structure. Even though this works it is all rather dodgy, be safe and check the return of i915_request_mark_eio before calling i915_request_put. Signed-off-by: Matthew Brost --- drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 6 -- drivers/gpu/drm/i915/gt/intel_ring_submission.c | 3 ++- drivers/gpu/drm/i915/gt/mock_engine.c| 3 ++- drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c| 9 ++--- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index ca03880fa7e4..fb7e64f78722 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -3073,7 +3073,8 @@ static void execlists_reset_cancel(struct intel_engine_cs *engine) /* Mark all executing requests as skipped. */ list_for_each_entry(rq, >sched_engine->requests, sched.link) - i915_request_put(i915_request_mark_eio(rq)); + if (i915_request_mark_eio(rq)) + i915_request_put(rq); intel_engine_signal_breadcrumbs(engine); /* Flush the queued requests to the timeline list (for retiring). */ @@ -3093,7 +3094,8 @@ static void execlists_reset_cancel(struct intel_engine_cs *engine) /* On-hold requests will be flushed to timeline upon their release */ list_for_each_entry(rq, _engine->hold, sched.link) - i915_request_put(i915_request_mark_eio(rq)); + if (i915_request_mark_eio(rq)) + i915_request_put(rq); /* Cancel all attached virtual engines */ while ((rb = rb_first_cached(>virtual))) { diff --git a/drivers/gpu/drm/i915/gt/intel_ring_submission.c b/drivers/gpu/drm/i915/gt/intel_ring_submission.c index 586dca1731ce..fc73f8744758 100644 --- a/drivers/gpu/drm/i915/gt/intel_ring_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_ring_submission.c @@ -419,7 +419,8 @@ static void reset_cancel(struct intel_engine_cs *engine) /* Mark all submitted requests as skipped. */ list_for_each_entry(request, >sched_engine->requests, sched.link) - i915_request_put(i915_request_mark_eio(request)); + if (i915_request_mark_eio(request)) + i915_request_put(request); intel_engine_signal_breadcrumbs(engine); /* Remaining _unready_ requests will be nop'ed when submitted */ diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c index 8b89215afe46..aae5e8dadd26 100644 --- a/drivers/gpu/drm/i915/gt/mock_engine.c +++ b/drivers/gpu/drm/i915/gt/mock_engine.c @@ -285,7 +285,8 @@ static void mock_reset_cancel(struct intel_engine_cs *engine) /* Mark all submitted requests as skipped. */ list_for_each_entry(rq, >sched_engine->requests, sched.link) - i915_request_put(i915_request_mark_eio(rq)); + if (i915_request_mark_eio(rq)) + i915_request_put(rq); intel_engine_signal_breadcrumbs(engine); /* Cancel and submit all pending requests. */ diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index d7e49f7c1dba..42bd2a8c6751 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -642,7 +642,8 @@ static int __guc_add_request(struct intel_guc *guc, struct i915_request *rq) * request resubmitted after the context was banned. */ if (unlikely(intel_context_is_banned(ce))) { - i915_request_put(i915_request_mark_eio(rq)); + if (i915_request_mark_eio(rq)) + i915_request_put(rq); intel_engine_signal_breadcrumbs(ce->engine); return 0; } @@ -1638,7 +1639,8 @@ static void guc_cancel_context_requests(struct intel_context *ce) spin_lock_irqsave(_engine->lock, flags); spin_lock(>guc_state.lock); list_for_each_entry(rq, >guc_state.requests, sched.link) - i915_request_put(i915_request_mark_eio(rq)); + if (i915_request_mark_eio(rq)) + i915_request_put(rq); spin_unlock(>guc_state.lock); spin_unlock_irqrestore(_engine->lock, flags); } @@ -1679,7 +1681,8 @@ guc_cancel_sched_engine_requests(struct i915_sched_engine *sched_engine) __i915_request_submit(rq); - i915_request_put(i915_request_mark_eio(rq)); + if
[PATCH] drm/i915: Drop stealing of bits from i915_sw_fence function pointer
Rather than stealing bits from i915_sw_fence function pointer use seperate fields for function pointer and flags. If using two different fields, the 4 byte alignment for the i915_sw_fence function pointer can also be dropped. v2: (CI) - Set new function field rather than flags in __i915_sw_fence_init v3: (Tvrtko) - Remove BUG_ON(!fence->flags) in reinit as that will now blow up - Only define fence->flags if CONFIG_DRM_I915_SW_FENCE_CHECK_DAG is defined v4: - Rebase, resend for CI Signed-off-by: Matthew Brost Acked-by: Jani Nikula Reviewed-by: Alan Previn --- drivers/gpu/drm/i915/display/intel_display.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_context.c | 2 +- drivers/gpu/drm/i915/gt/intel_context.c | 2 +- drivers/gpu/drm/i915/i915_request.c | 4 +-- drivers/gpu/drm/i915/i915_sw_fence.c | 28 +++ drivers/gpu/drm/i915/i915_sw_fence.h | 23 +++ drivers/gpu/drm/i915/i915_sw_fence_work.c | 2 +- .../gpu/drm/i915/selftests/i915_sw_fence.c| 2 +- drivers/gpu/drm/i915/selftests/lib_sw_fence.c | 8 +++--- 9 files changed, 40 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 0ceee8ac6671..6636ac9e9ff8 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -8791,7 +8791,7 @@ static void intel_atomic_commit_work(struct work_struct *work) intel_atomic_commit_tail(state); } -static int __i915_sw_fence_call +static int intel_atomic_commit_ready(struct i915_sw_fence *fence, enum i915_sw_fence_notify notify) { diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index ebd775cb1661..347dab952e90 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -1001,7 +1001,7 @@ static void free_engines_rcu(struct rcu_head *rcu) free_engines(engines); } -static int __i915_sw_fence_call +static int engines_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state) { struct i915_gem_engines *engines = diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c index 5634d14052bc..331ed688ceab 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.c +++ b/drivers/gpu/drm/i915/gt/intel_context.c @@ -364,7 +364,7 @@ static int __intel_context_active(struct i915_active *active) return 0; } -static int __i915_sw_fence_call +static int sw_fence_dummy_notify(struct i915_sw_fence *sf, enum i915_sw_fence_notify state) { diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 623273aca09e..08dda8889f4a 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -719,7 +719,7 @@ void i915_request_cancel(struct i915_request *rq, int error) intel_context_cancel_request(rq->context, rq); } -static int __i915_sw_fence_call +static int submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state) { struct i915_request *request = @@ -755,7 +755,7 @@ submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state) return NOTIFY_DONE; } -static int __i915_sw_fence_call +static int semaphore_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state) { struct i915_request *rq = container_of(fence, typeof(*rq), semaphore); diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c index c589a681da77..f10d31818ecc 100644 --- a/drivers/gpu/drm/i915/i915_sw_fence.c +++ b/drivers/gpu/drm/i915/i915_sw_fence.c @@ -18,7 +18,9 @@ #define I915_SW_FENCE_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr) #endif +#ifdef CONFIG_DRM_I915_SW_FENCE_CHECK_DAG static DEFINE_SPINLOCK(i915_sw_fence_lock); +#endif #define WQ_FLAG_BITS \ BITS_PER_TYPE(typeof_member(struct wait_queue_entry, flags)) @@ -34,7 +36,7 @@ enum { static void *i915_sw_fence_debug_hint(void *addr) { - return (void *)(((struct i915_sw_fence *)addr)->flags & I915_SW_FENCE_MASK); + return (void *)(((struct i915_sw_fence *)addr)->fn); } #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS @@ -126,10 +128,7 @@ static inline void debug_fence_assert(struct i915_sw_fence *fence) static int __i915_sw_fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state) { - i915_sw_fence_notify_t fn; - - fn = (i915_sw_fence_notify_t)(fence->flags & I915_SW_FENCE_MASK); - return fn(fence, state); + return fence->fn(fence, state); } #ifdef CONFIG_DRM_I915_SW_FENCE_DEBUG_OBJECTS @@ -242,10 +241,13 @@ void __i915_sw_fence_init(struct i915_sw_fence *fence, const char *name, struct lock_class_key *key) { -
Re: [PATCH v3 1/5] drm/i915/dg2: s/DISP_STEPPING/DISPLAY_STEPPING/
On Tue, 16 Nov 2021, Matt Roper wrote: > Commit cd0fcf5af791 ("drm/i915: rename DISP_STEPPING->DISPLAY_STEP and > GT_STEPPING->GT_STEP") renamed all platforms' display stepping tests, > but the DG2 patches were still in-flight at that time and did not > incorporate the new naming scheme. Rename DG2's macro now for > consistency with other platforms. > > Cc: Jani Nikula > Signed-off-by: Matt Roper Reviewed-by: Jani Nikula > --- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 6f9f20a10c0c..b3404041294d 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1598,7 +1598,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, > (IS_SUBPLATFORM(__i915, INTEL_DG2, INTEL_SUBPLATFORM_##variant) && \ >IS_GRAPHICS_STEP(__i915, since, until)) > > -#define IS_DG2_DISP_STEP(__i915, since, until) \ > +#define IS_DG2_DISPLAY_STEP(__i915, since, until) \ > (IS_DG2(__i915) && \ >IS_DISPLAY_STEP(__i915, since, until)) -- Jani Nikula, Intel Open Source Graphics Center
[PATCH AUTOSEL 5.15 08/65] drm/virtio: fix the missed drm_gem_object_put() in virtio_gpu_user_framebuffer_create()
From: Jing Xiangfeng [ Upstream commit a63f393dd7e1ebee707c9dee1d197fdc33d6486b ] virtio_gpu_user_framebuffer_create() misses to call drm_gem_object_put() in an error path. Add the missed function call to fix it. Signed-off-by: Jing Xiangfeng Link: http://patchwork.freedesktop.org/patch/msgid/1633770560-11658-1-git-send-email-jingxiangf...@huawei.com Signed-off-by: Gerd Hoffmann Signed-off-by: Sasha Levin --- drivers/gpu/drm/virtio/virtgpu_display.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c index a6caebd4a0dd6..5b00310ac4cd4 100644 --- a/drivers/gpu/drm/virtio/virtgpu_display.c +++ b/drivers/gpu/drm/virtio/virtgpu_display.c @@ -308,8 +308,10 @@ virtio_gpu_user_framebuffer_create(struct drm_device *dev, return ERR_PTR(-EINVAL); virtio_gpu_fb = kzalloc(sizeof(*virtio_gpu_fb), GFP_KERNEL); - if (virtio_gpu_fb == NULL) + if (virtio_gpu_fb == NULL) { + drm_gem_object_put(obj); return ERR_PTR(-ENOMEM); + } ret = virtio_gpu_framebuffer_init(dev, virtio_gpu_fb, mode_cmd, obj); if (ret) { -- 2.33.0
[PATCH AUTOSEL 5.15 07/65] fbdev: fbmem: Fix double free of 'fb_info->pixmap.addr'
From: Zheyu Ma [ Upstream commit 2c0c19b681d5a331b53aab0d170f72a87c7bff12 ] savagefb and some other drivers call kfree to free 'info->pixmap.addr' even after calling unregister_framebuffer, which may cause double free. Fix this by setting 'fb_info->pixmap.addr' to NULL after kfree in unregister_framebuffer. The following log reveals it: [ 37.318872] BUG: KASAN: double-free or invalid-free in kfree+0x13e/0x290 [ 37.319369] [ 37.320803] Call Trace: [ 37.320992] dump_stack_lvl+0xa8/0xd1 [ 37.321274] print_address_description+0x87/0x3b0 [ 37.321632] ? kfree+0x13e/0x290 [ 37.321879] ? kfree+0x13e/0x290 [ 37.322126] ? kfree+0x13e/0x290 [ 37.322374] kasan_report_invalid_free+0x58/0x90 [ 37.322724] kasan_slab_free+0x123/0x140 [ 37.323049] __kasan_slab_free+0x11/0x20 [ 37.323347] slab_free_freelist_hook+0x81/0x150 [ 37.323689] ? savagefb_remove+0xa1/0xc0 [savagefb] [ 37.324066] kfree+0x13e/0x290 [ 37.324304] savagefb_remove+0xa1/0xc0 [savagefb] [ 37.324655] pci_device_remove+0xa9/0x250 [ 37.324959] ? pci_device_probe+0x7d0/0x7d0 [ 37.325273] device_release_driver_internal+0x4f7/0x7a0 [ 37.325666] driver_detach+0x1e8/0x2c0 [ 37.325952] bus_remove_driver+0x134/0x290 [ 37.326262] ? sysfs_remove_groups+0x97/0xb0 [ 37.326584] driver_unregister+0x77/0xa0 [ 37.326883] pci_unregister_driver+0x2c/0x1c0 [ 37.336124] [ 37.336245] Allocated by task 5465: [ 37.336507] kasan_kmalloc+0xb5/0xe0 [ 37.336801] __kasan_kmalloc+0x9/0x10 [ 37.337069] kmem_cache_alloc_trace+0x12b/0x220 [ 37.337405] register_framebuffer+0x3f3/0xa00 [ 37.337731] foo_register_framebuffer+0x3b/0x50 [savagefb] [ 37.338136] [ 37.338255] Freed by task 5475: [ 37.338492] kasan_set_track+0x3d/0x70 [ 37.338774] kasan_set_free_info+0x23/0x40 [ 37.339081] kasan_slab_free+0x10b/0x140 [ 37.339399] __kasan_slab_free+0x11/0x20 [ 37.339694] slab_free_freelist_hook+0x81/0x150 [ 37.340034] kfree+0x13e/0x290 [ 37.340267] do_unregister_framebuffer+0x21c/0x3d0 [ 37.340624] unregister_framebuffer+0x23/0x40 [ 37.340950] savagefb_remove+0x45/0xc0 [savagefb] [ 37.341302] pci_device_remove+0xa9/0x250 [ 37.341603] device_release_driver_internal+0x4f7/0x7a0 [ 37.341990] driver_detach+0x1e8/0x2c0 [ 37.342272] bus_remove_driver+0x134/0x290 [ 37.342577] driver_unregister+0x77/0xa0 [ 37.342873] pci_unregister_driver+0x2c/0x1c0 [ 37.343196] cleanup_module+0x15/0x1c [savagefb] [ 37.343543] __se_sys_delete_module+0x398/0x490 [ 37.343881] __x64_sys_delete_module+0x56/0x60 [ 37.344221] do_syscall_64+0x4d/0xc0 [ 37.344492] entry_SYSCALL_64_after_hwframe+0x44/0xae Signed-off-by: Zheyu Ma Signed-off-by: Sam Ravnborg Link: https://patchwork.freedesktop.org/patch/msgid/1633848148-29747-1-git-send-email-zheyum...@gmail.com Signed-off-by: Sasha Levin --- drivers/video/fbdev/core/fbmem.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 7420d2c16e47e..826175ad88a2f 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1702,8 +1702,11 @@ static void do_unregister_framebuffer(struct fb_info *fb_info) { unlink_framebuffer(fb_info); if (fb_info->pixmap.addr && - (fb_info->pixmap.flags & FB_PIXMAP_DEFAULT)) + (fb_info->pixmap.flags & FB_PIXMAP_DEFAULT)) { kfree(fb_info->pixmap.addr); + fb_info->pixmap.addr = NULL; + } + fb_destroy_modelist(_info->modelist); registered_fb[fb_info->node] = NULL; num_registered_fb--; -- 2.33.0
[PATCH AUTOSEL 5.15 03/65] backlight: Propagate errors from get_brightness()
From: Thomas Weißschuh [ Upstream commit 563edf85ce18a90dd0a7b39e279a691d937205f6 ] backlight.h documents "struct backlight_ops->get_brightness()" to return a negative errno on failure. So far these errors have not been handled in the backlight core. This leads to negative values being exposed through sysfs although only positive values are documented to be reported. Signed-off-by: Thomas Weißschuh Reviewed-by: Daniel Thompson Signed-off-by: Lee Jones Signed-off-by: Sasha Levin --- drivers/video/backlight/backlight.c | 22 +- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c index 537fe1b376ad7..4658cfb75aa28 100644 --- a/drivers/video/backlight/backlight.c +++ b/drivers/video/backlight/backlight.c @@ -292,10 +292,13 @@ static ssize_t actual_brightness_show(struct device *dev, struct backlight_device *bd = to_backlight_device(dev); mutex_lock(>ops_lock); - if (bd->ops && bd->ops->get_brightness) - rc = sprintf(buf, "%d\n", bd->ops->get_brightness(bd)); - else + if (bd->ops && bd->ops->get_brightness) { + rc = bd->ops->get_brightness(bd); + if (rc >= 0) + rc = sprintf(buf, "%d\n", rc); + } else { rc = sprintf(buf, "%d\n", bd->props.brightness); + } mutex_unlock(>ops_lock); return rc; @@ -381,9 +384,18 @@ ATTRIBUTE_GROUPS(bl_device); void backlight_force_update(struct backlight_device *bd, enum backlight_update_reason reason) { + int brightness; + mutex_lock(>ops_lock); - if (bd->ops && bd->ops->get_brightness) - bd->props.brightness = bd->ops->get_brightness(bd); + if (bd->ops && bd->ops->get_brightness) { + brightness = bd->ops->get_brightness(bd); + if (brightness >= 0) + bd->props.brightness = brightness; + else + dev_err(>dev, + "Could not update brightness from device: %pe\n", + ERR_PTR(brightness)); + } mutex_unlock(>ops_lock); backlight_generate_event(bd, reason); } -- 2.33.0
[PATCH AUTOSEL 5.15 08/65] drm/virtio: fix the missed drm_gem_object_put() in virtio_gpu_user_framebuffer_create()
From: Jing Xiangfeng [ Upstream commit a63f393dd7e1ebee707c9dee1d197fdc33d6486b ] virtio_gpu_user_framebuffer_create() misses to call drm_gem_object_put() in an error path. Add the missed function call to fix it. Signed-off-by: Jing Xiangfeng Link: http://patchwork.freedesktop.org/patch/msgid/1633770560-11658-1-git-send-email-jingxiangf...@huawei.com Signed-off-by: Gerd Hoffmann Signed-off-by: Sasha Levin --- drivers/gpu/drm/virtio/virtgpu_display.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c index a6caebd4a0dd6..5b00310ac4cd4 100644 --- a/drivers/gpu/drm/virtio/virtgpu_display.c +++ b/drivers/gpu/drm/virtio/virtgpu_display.c @@ -308,8 +308,10 @@ virtio_gpu_user_framebuffer_create(struct drm_device *dev, return ERR_PTR(-EINVAL); virtio_gpu_fb = kzalloc(sizeof(*virtio_gpu_fb), GFP_KERNEL); - if (virtio_gpu_fb == NULL) + if (virtio_gpu_fb == NULL) { + drm_gem_object_put(obj); return ERR_PTR(-ENOMEM); + } ret = virtio_gpu_framebuffer_init(dev, virtio_gpu_fb, mode_cmd, obj); if (ret) { -- 2.33.0
[PATCH AUTOSEL 5.15 07/65] fbdev: fbmem: Fix double free of 'fb_info->pixmap.addr'
From: Zheyu Ma [ Upstream commit 2c0c19b681d5a331b53aab0d170f72a87c7bff12 ] savagefb and some other drivers call kfree to free 'info->pixmap.addr' even after calling unregister_framebuffer, which may cause double free. Fix this by setting 'fb_info->pixmap.addr' to NULL after kfree in unregister_framebuffer. The following log reveals it: [ 37.318872] BUG: KASAN: double-free or invalid-free in kfree+0x13e/0x290 [ 37.319369] [ 37.320803] Call Trace: [ 37.320992] dump_stack_lvl+0xa8/0xd1 [ 37.321274] print_address_description+0x87/0x3b0 [ 37.321632] ? kfree+0x13e/0x290 [ 37.321879] ? kfree+0x13e/0x290 [ 37.322126] ? kfree+0x13e/0x290 [ 37.322374] kasan_report_invalid_free+0x58/0x90 [ 37.322724] kasan_slab_free+0x123/0x140 [ 37.323049] __kasan_slab_free+0x11/0x20 [ 37.323347] slab_free_freelist_hook+0x81/0x150 [ 37.323689] ? savagefb_remove+0xa1/0xc0 [savagefb] [ 37.324066] kfree+0x13e/0x290 [ 37.324304] savagefb_remove+0xa1/0xc0 [savagefb] [ 37.324655] pci_device_remove+0xa9/0x250 [ 37.324959] ? pci_device_probe+0x7d0/0x7d0 [ 37.325273] device_release_driver_internal+0x4f7/0x7a0 [ 37.325666] driver_detach+0x1e8/0x2c0 [ 37.325952] bus_remove_driver+0x134/0x290 [ 37.326262] ? sysfs_remove_groups+0x97/0xb0 [ 37.326584] driver_unregister+0x77/0xa0 [ 37.326883] pci_unregister_driver+0x2c/0x1c0 [ 37.336124] [ 37.336245] Allocated by task 5465: [ 37.336507] kasan_kmalloc+0xb5/0xe0 [ 37.336801] __kasan_kmalloc+0x9/0x10 [ 37.337069] kmem_cache_alloc_trace+0x12b/0x220 [ 37.337405] register_framebuffer+0x3f3/0xa00 [ 37.337731] foo_register_framebuffer+0x3b/0x50 [savagefb] [ 37.338136] [ 37.338255] Freed by task 5475: [ 37.338492] kasan_set_track+0x3d/0x70 [ 37.338774] kasan_set_free_info+0x23/0x40 [ 37.339081] kasan_slab_free+0x10b/0x140 [ 37.339399] __kasan_slab_free+0x11/0x20 [ 37.339694] slab_free_freelist_hook+0x81/0x150 [ 37.340034] kfree+0x13e/0x290 [ 37.340267] do_unregister_framebuffer+0x21c/0x3d0 [ 37.340624] unregister_framebuffer+0x23/0x40 [ 37.340950] savagefb_remove+0x45/0xc0 [savagefb] [ 37.341302] pci_device_remove+0xa9/0x250 [ 37.341603] device_release_driver_internal+0x4f7/0x7a0 [ 37.341990] driver_detach+0x1e8/0x2c0 [ 37.342272] bus_remove_driver+0x134/0x290 [ 37.342577] driver_unregister+0x77/0xa0 [ 37.342873] pci_unregister_driver+0x2c/0x1c0 [ 37.343196] cleanup_module+0x15/0x1c [savagefb] [ 37.343543] __se_sys_delete_module+0x398/0x490 [ 37.343881] __x64_sys_delete_module+0x56/0x60 [ 37.344221] do_syscall_64+0x4d/0xc0 [ 37.344492] entry_SYSCALL_64_after_hwframe+0x44/0xae Signed-off-by: Zheyu Ma Signed-off-by: Sam Ravnborg Link: https://patchwork.freedesktop.org/patch/msgid/1633848148-29747-1-git-send-email-zheyum...@gmail.com Signed-off-by: Sasha Levin --- drivers/video/fbdev/core/fbmem.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 7420d2c16e47e..826175ad88a2f 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1702,8 +1702,11 @@ static void do_unregister_framebuffer(struct fb_info *fb_info) { unlink_framebuffer(fb_info); if (fb_info->pixmap.addr && - (fb_info->pixmap.flags & FB_PIXMAP_DEFAULT)) + (fb_info->pixmap.flags & FB_PIXMAP_DEFAULT)) { kfree(fb_info->pixmap.addr); + fb_info->pixmap.addr = NULL; + } + fb_destroy_modelist(_info->modelist); registered_fb[fb_info->node] = NULL; num_registered_fb--; -- 2.33.0
[PATCH AUTOSEL 5.15 03/65] backlight: Propagate errors from get_brightness()
From: Thomas Weißschuh [ Upstream commit 563edf85ce18a90dd0a7b39e279a691d937205f6 ] backlight.h documents "struct backlight_ops->get_brightness()" to return a negative errno on failure. So far these errors have not been handled in the backlight core. This leads to negative values being exposed through sysfs although only positive values are documented to be reported. Signed-off-by: Thomas Weißschuh Reviewed-by: Daniel Thompson Signed-off-by: Lee Jones Signed-off-by: Sasha Levin --- drivers/video/backlight/backlight.c | 22 +- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c index 537fe1b376ad7..4658cfb75aa28 100644 --- a/drivers/video/backlight/backlight.c +++ b/drivers/video/backlight/backlight.c @@ -292,10 +292,13 @@ static ssize_t actual_brightness_show(struct device *dev, struct backlight_device *bd = to_backlight_device(dev); mutex_lock(>ops_lock); - if (bd->ops && bd->ops->get_brightness) - rc = sprintf(buf, "%d\n", bd->ops->get_brightness(bd)); - else + if (bd->ops && bd->ops->get_brightness) { + rc = bd->ops->get_brightness(bd); + if (rc >= 0) + rc = sprintf(buf, "%d\n", rc); + } else { rc = sprintf(buf, "%d\n", bd->props.brightness); + } mutex_unlock(>ops_lock); return rc; @@ -381,9 +384,18 @@ ATTRIBUTE_GROUPS(bl_device); void backlight_force_update(struct backlight_device *bd, enum backlight_update_reason reason) { + int brightness; + mutex_lock(>ops_lock); - if (bd->ops && bd->ops->get_brightness) - bd->props.brightness = bd->ops->get_brightness(bd); + if (bd->ops && bd->ops->get_brightness) { + brightness = bd->ops->get_brightness(bd); + if (brightness >= 0) + bd->props.brightness = brightness; + else + dev_err(>dev, + "Could not update brightness from device: %pe\n", + ERR_PTR(brightness)); + } mutex_unlock(>ops_lock); backlight_generate_event(bd, reason); } -- 2.33.0
Re: [PATCH] drm/scheduler: fix drm_sched_job_add_implicit_dependencies harder
On Tue, 16 Nov 2021 at 21:21, Rob Clark wrote: > > From: Rob Clark > > drm_sched_job_add_dependency() could drop the last ref, so we need to do > the dma_fence_get() first. > It fixed the splats I saw on RB5 (sm8250 | A650). Thanks. Tested-by: Amit Pundir > Cc: Christian König > Fixes: 9c2ba265352a drm/scheduler: ("use new iterator in > drm_sched_job_add_implicit_dependencies v2") > Signed-off-by: Rob Clark > --- > Applies on top of "drm/scheduler: fix drm_sched_job_add_implicit_dependencies" > but I don't think that has a stable commit sha yet. > > drivers/gpu/drm/scheduler/sched_main.c | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/scheduler/sched_main.c > b/drivers/gpu/drm/scheduler/sched_main.c > index 94fe51b3caa2..f91fb31ab7a7 100644 > --- a/drivers/gpu/drm/scheduler/sched_main.c > +++ b/drivers/gpu/drm/scheduler/sched_main.c > @@ -704,12 +704,13 @@ int drm_sched_job_add_implicit_dependencies(struct > drm_sched_job *job, > int ret; > > dma_resv_for_each_fence(, obj->resv, write, fence) { > - ret = drm_sched_job_add_dependency(job, fence); > - if (ret) > - return ret; > - > /* Make sure to grab an additional ref on the added fence */ > dma_fence_get(fence); > + ret = drm_sched_job_add_dependency(job, fence); > + if (ret) { > + dma_fence_put(fence); > + return ret; > + } > } > return 0; > } > -- > 2.33.1 >
Re: [PATCH 2/2] drm/sched: fix dropping the last fence ref
On Tue, Nov 16, 2021 at 8:37 AM Daniel Vetter wrote: > > On Tue, Nov 16, 2021 at 10:25:19AM +0100, Christian König wrote: > > We need to grab another ref before trying to add the fence to the sched > > job and not after. > > > > Signed-off-by: Christian König > > Reviewed-by: Daniel Vetter > > I wondered first why this goes boom, but then I realized that in some > cases add_dependency() drops the reference of the passed-in fence. > > Please also add the Fixes: line like in the previous patch. oh, I sent https://patchwork.freedesktop.org/patch/463329/ before I saw this.. it already has the fixes tag, and IMO a better description, so I think you can just pick that one instead BR, -R > Cheers, Daniel > > > --- > > drivers/gpu/drm/scheduler/sched_main.c | 10 ++ > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/scheduler/sched_main.c > > b/drivers/gpu/drm/scheduler/sched_main.c > > index 94fe51b3caa2..400d201c3c28 100644 > > --- a/drivers/gpu/drm/scheduler/sched_main.c > > +++ b/drivers/gpu/drm/scheduler/sched_main.c > > @@ -704,12 +704,14 @@ int drm_sched_job_add_implicit_dependencies(struct > > drm_sched_job *job, > > int ret; > > > > dma_resv_for_each_fence(, obj->resv, write, fence) { > > - ret = drm_sched_job_add_dependency(job, fence); > > - if (ret) > > - return ret; > > - > > /* Make sure to grab an additional ref on the added fence */ > > dma_fence_get(fence); > > + > > + ret = drm_sched_job_add_dependency(job, fence); > > + if (ret) { > > + dma_fence_put(fence); > > + return ret; > > + } > > } > > return 0; > > } > > -- > > 2.25.1 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
[PATCH v3 5/5] drm/i915/dg2: extend Wa_1409120013 to DG2
From: Matt Atwood Extend existing workaround 1409120013 to DG2. Cc: José Roberto de Souza Signed-off-by: Matt Atwood Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/intel_pm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 89dc7f69baf3..e721c421cc58 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -7444,9 +7444,9 @@ static void icl_init_clock_gating(struct drm_i915_private *dev_priv) static void gen12lp_init_clock_gating(struct drm_i915_private *dev_priv) { - /* Wa_1409120013:tgl,rkl,adl-s,dg1 */ + /* Wa_1409120013:tgl,rkl,adl-s,dg1,dg2 */ if (IS_TIGERLAKE(dev_priv) || IS_ROCKETLAKE(dev_priv) || - IS_ALDERLAKE_S(dev_priv) || IS_DG1(dev_priv)) + IS_ALDERLAKE_S(dev_priv) || IS_DG1(dev_priv) || IS_DG2(dev_priv)) intel_uncore_write(_priv->uncore, ILK_DPFC_CHICKEN, DPFC_CHICKEN_COMP_DUMMY_PIXEL); -- 2.33.0
[PATCH v3 2/5] drm/i915/dg2: Add Wa_14010547955
This workaround is documented a bit strangely in the bspec; it's listed as an A0 workaround, but the description clarifies that the workaround is implicitly handled by the hardware and what the driver really needs to do is program a chicken bit to reenable some internal behavior. Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/display/intel_display.c | 4 drivers/gpu/drm/i915/i915_reg.h | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 0ceee8ac6671..1639bdbe2091 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -988,6 +988,10 @@ static void icl_set_pipe_chicken(const struct intel_crtc_state *crtc_state) else if (DISPLAY_VER(dev_priv) >= 13) tmp |= UNDERRUN_RECOVERY_DISABLE_ADLP; + /* Wa_14010547955:dg2 */ + if (IS_DG2_DISPLAY_STEP(dev_priv, STEP_B0, STEP_FOREVER)) + tmp |= DG2_RENDER_CCSTAG_4_3_EN; + intel_de_write(dev_priv, PIPE_CHICKEN(pipe), tmp); } diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index f15ffc53e858..c187ec122fdb 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -8568,8 +8568,9 @@ enum { _PIPEB_CHICKEN) #define UNDERRUN_RECOVERY_DISABLE_ADLP REG_BIT(30) #define UNDERRUN_RECOVERY_ENABLE_DG2 REG_BIT(30) -#define PIXEL_ROUNDING_TRUNC_FB_PASSTHRU (1 << 15) -#define PER_PIXEL_ALPHA_BYPASS_EN(1 << 7) +#define PIXEL_ROUNDING_TRUNC_FB_PASSTHRU REG_BIT(15) +#define DG2_RENDER_CCSTAG_4_3_EN REG_BIT(12) +#define PER_PIXEL_ALPHA_BYPASS_ENREG_BIT(7) #define VFLSKPD_MMIO(0x62a8) #define DIS_OVER_FETCH_CACHE REG_BIT(1) -- 2.33.0
[PATCH v3 3/5] drm/i915/dg2: Add Wa_16011777198
Coarse power gating for render should not be enabled on some DG2 steppings. Bspec: 52698 Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/gt/intel_rc6.c | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c index 43093dd2d0c9..c3155ee58689 100644 --- a/drivers/gpu/drm/i915/gt/intel_rc6.c +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c @@ -117,10 +117,17 @@ static void gen11_rc6_enable(struct intel_rc6 *rc6) GEN6_RC_CTL_RC6_ENABLE | GEN6_RC_CTL_EI_MODE(1); - pg_enable = - GEN9_RENDER_PG_ENABLE | - GEN9_MEDIA_PG_ENABLE | - GEN11_MEDIA_SAMPLER_PG_ENABLE; + /* Wa_16011777198 - Render powergating must remain disabled */ + if (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) || + IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0)) + pg_enable = + GEN9_MEDIA_PG_ENABLE | + GEN11_MEDIA_SAMPLER_PG_ENABLE; + else + pg_enable = + GEN9_RENDER_PG_ENABLE | + GEN9_MEDIA_PG_ENABLE | + GEN11_MEDIA_SAMPLER_PG_ENABLE; if (GRAPHICS_VER(gt->i915) >= 12) { for (i = 0; i < I915_MAX_VCS; i++) -- 2.33.0
[PATCH v3 4/5] drm/i915/dg2: Add Wa_16013000631
From: Ramalingam C Invalidate IC cache through pipe control command as part of the ctx restore flow through indirect ctx pointer. v2: - Move pipe control from xcs indirect context to the rcs indirect context. We'll eventually need this on the CCS engines too, but support for those hasn't landed yet. Cc: Chris Wilson Signed-off-by: Ramalingam C Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/gt/intel_lrc.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index 56156cf18c41..b3489599e4de 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -1167,6 +1167,11 @@ gen12_emit_indirect_ctx_rcs(const struct intel_context *ce, u32 *cs) cs = gen12_emit_cmd_buf_wa(ce, cs); cs = gen12_emit_restore_scratch(ce, cs); + /* Wa_16013000631:dg2 */ + if (IS_DG2_GRAPHICS_STEP(ce->engine->i915, G10, STEP_B0, STEP_C0) || + IS_DG2_G11(ce->engine->i915)) + cs = gen8_emit_pipe_control(cs, PIPE_CONTROL_INSTRUCTION_CACHE_INVALIDATE, 0); + return cs; } -- 2.33.0
[PATCH v3 0/5] i915: Additional DG2 workarounds
We have a few more DG2 workarounds that weren't included in the initial batch. v2: - Apply pipe control workaround to rcs indirect context rather than xcs indirect context. v3: - s/IS_DG2_DISP_STEP/IS_DG2_DISPLAY_STEPPING/ for consistency with other platforms. Matt Atwood (1): drm/i915/dg2: extend Wa_1409120013 to DG2 Matt Roper (3): drm/i915/dg2: s/DISP_STEPPING/DISPLAY_STEPPING/ drm/i915/dg2: Add Wa_14010547955 drm/i915/dg2: Add Wa_16011777198 Ramalingam C (1): drm/i915/dg2: Add Wa_16013000631 drivers/gpu/drm/i915/display/intel_display.c | 4 drivers/gpu/drm/i915/gt/intel_lrc.c | 5 + drivers/gpu/drm/i915/gt/intel_rc6.c | 15 +++ drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/i915_reg.h | 5 +++-- drivers/gpu/drm/i915/intel_pm.c | 4 ++-- 6 files changed, 26 insertions(+), 9 deletions(-) -- 2.33.0
[PATCH v3 1/5] drm/i915/dg2: s/DISP_STEPPING/DISPLAY_STEPPING/
Commit cd0fcf5af791 ("drm/i915: rename DISP_STEPPING->DISPLAY_STEP and GT_STEPPING->GT_STEP") renamed all platforms' display stepping tests, but the DG2 patches were still in-flight at that time and did not incorporate the new naming scheme. Rename DG2's macro now for consistency with other platforms. Cc: Jani Nikula Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/i915_drv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 6f9f20a10c0c..b3404041294d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1598,7 +1598,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, (IS_SUBPLATFORM(__i915, INTEL_DG2, INTEL_SUBPLATFORM_##variant) && \ IS_GRAPHICS_STEP(__i915, since, until)) -#define IS_DG2_DISP_STEP(__i915, since, until) \ +#define IS_DG2_DISPLAY_STEP(__i915, since, until) \ (IS_DG2(__i915) && \ IS_DISPLAY_STEP(__i915, since, until)) -- 2.33.0
[PATCH v4] drm/msm/dp: employ bridge mechanism for display enable and disable
Currently the msm_dp_*** functions implement the same sequence which would happen when drm_bridge is used. hence get rid of this intermediate layer and align with the drm_bridge usage to avoid customized implementation. Signed-off-by: Kuogee Hsieh Changes in v2: -- revise commit text -- rename dp_bridge to msm_dp_bridge -- delete empty functions Changes in v3: -- replace kzalloc() with devm_kzalloc() -- replace __dp_display_enable() with dp_display_enable() -- replace __dp_display_disable() with dp_display_disable() Changes in v4: -- msm_dp_bridge_init() called from msm_dp_modeset_init() same as dsi --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 21 --- drivers/gpu/drm/msm/dp/dp_display.c | 16 +- drivers/gpu/drm/msm/dp/dp_display.h | 1 + drivers/gpu/drm/msm/dp/dp_drm.c | 86 + drivers/gpu/drm/msm/msm_drv.h | 18 -- 5 files changed, 115 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 31050aa..c4e08c4 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1003,9 +1003,6 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc, trace_dpu_enc_mode_set(DRMID(drm_enc)); - if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS) - msm_dp_display_mode_set(dpu_enc->dp, drm_enc, mode, adj_mode); - list_for_each_entry(conn_iter, connector_list, head) if (conn_iter->encoder == drm_enc) conn = conn_iter; @@ -1181,14 +1178,6 @@ static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc) _dpu_encoder_virt_enable_helper(drm_enc); - if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS) { - ret = msm_dp_display_enable(dpu_enc->dp, drm_enc); - if (ret) { - DPU_ERROR_ENC(dpu_enc, "dp display enable failed: %d\n", - ret); - goto out; - } - } dpu_enc->enabled = true; out: @@ -1214,11 +1203,6 @@ static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc) /* wait for idle */ dpu_encoder_wait_for_event(drm_enc, MSM_ENC_TX_COMPLETE); - if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS) { - if (msm_dp_display_pre_disable(dpu_enc->dp, drm_enc)) - DPU_ERROR_ENC(dpu_enc, "dp display push idle failed\n"); - } - dpu_encoder_resource_control(drm_enc, DPU_ENC_RC_EVENT_PRE_STOP); for (i = 0; i < dpu_enc->num_phys_encs; i++) { @@ -1243,11 +1227,6 @@ static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc) DPU_DEBUG_ENC(dpu_enc, "encoder disabled\n"); - if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS) { - if (msm_dp_display_disable(dpu_enc->dp, drm_enc)) - DPU_ERROR_ENC(dpu_enc, "dp display disable failed\n"); - } - mutex_unlock(_enc->enc_lock); } diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 2f113ff..89a8d43 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -1571,6 +1571,18 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev, } priv->connectors[priv->num_connectors++] = dp_display->connector; + + dp_display->bridge = msm_dp_bridge_init(dp_display, dev, encoder); + if (IS_ERR(dp_display->bridge)) { + ret = PTR_ERR(dp_display->bridge); + DRM_DEV_ERROR(dev->dev, + "failed to create dp bridge: %d\n", ret); + dp_display->bridge = NULL; + return ret; + } + + priv->bridges[priv->num_bridges++] = dp_display->bridge; + return 0; } @@ -1674,8 +1686,8 @@ int msm_dp_display_disable(struct msm_dp *dp, struct drm_encoder *encoder) } void msm_dp_display_mode_set(struct msm_dp *dp, struct drm_encoder *encoder, - struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode) + const struct drm_display_mode *mode, + const struct drm_display_mode *adjusted_mode) { struct dp_display_private *dp_display; diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h index 76f45f9..2237e80 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.h +++ b/drivers/gpu/drm/msm/dp/dp_display.h @@ -13,6 +13,7 @@ struct msm_dp { struct drm_device *drm_dev; struct device *codec_dev; + struct drm_bridge *bridge; struct drm_connector *connector; struct drm_encoder *encoder; struct drm_panel *drm_panel; diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c
Re: [PATCH] drm: change logs to print connectors in the form CONNECTOR:id:name
On Mon, Nov 15, 2021 at 10:17:58PM +0200, Jani Nikula wrote: > On Mon, 15 Nov 2021, Claudio Suarez wrote: > > On Mon, Nov 15, 2021 at 12:24:26PM +0200, Jani Nikula wrote: > >> On Sun, 14 Nov 2021, Claudio Suarez wrote: > >> > On Sat, Nov 13, 2021 at 09:39:46PM +0100, Sam Ravnborg wrote: > >> >> Hi Claudio, > >> >> > >> >> On Sat, Nov 13, 2021 at 08:27:30PM +0100, Claudio Suarez wrote: > >> >> > The prefered way to log connectors is [CONNECTOR:id:name]. Change it > >> >> > in > >> >> > drm core programs. > >> >> > > >> >> > Suggested-by: Ville Syrjälä > >> >> > Signed-off-by: Claudio Suarez > >> >> > >> >> While touching all these logging calls, could you convernt them to the > >> >> newer drm_dbg_kms variants? > >> >> DRM_DEBUG_* are all deprecated. > >> >> > >> > > >> > Yes, I can, but it is recommended to do it in a different patch: > >> > > >> > https://www.kernel.org/doc/html/latest/process/submitting-patches.html#separate-your-changes > >> > > >> > C: > >> > "Separate your changes > >> > Separate each logical change into a separate patch. > >> > For example, if your changes include both bug fixes and performance > >> > enhancements..." > >> > > >> > > >> > I will study and send a new separate patch with your suggestion. > >> > >> I feel these logging changes are the types of changes where I'd err on > >> the side of fewer patches than strictly following the above guidelines. > > > > To size the problem: > > - there are about 3434 references to DRM_DEBUG_* in all the drm code, all > > drivers. > > - there are 413 references to DRM_DEBUG_* in the drm core code, only 66 of > >them are related to connectors. > > - there are 62 references to DRM_ERR* and 7 references to DRM_INFO in > >the drm core programs. > > > > I meant I can make two patches: > > 1 - this one with the change to CONNECTOR:id:name (29 changes) > > 2 - a new and bigger patch to change 413 + 62 + 7 references to > > DRM_{DEBUG,ERR,INFO} > > in the drm core programs. > > The second one is an on-going change that will have to happen gradually, > file by file. Changing connector references while at it seems like a > reasonable drive-by-change to me. (Others may disagree.) There are about 50 files = 50 patches. It seems excessive to me, but I get the point: smaller changes, so they are easier to control/review/... I am going so send the patch 1 and one of the patches 2 and we can see. Thanks, Claudio Suarez
Re: [PATCH] drm/i915: Fix error pointer dereference in i915_gem_do_execbuffer()
On Tue, Nov 16, 2021 at 02:48:17PM +0300, Dan Carpenter wrote: > Originally "out_fence" was set using out_fence = sync_file_create() but > which returns NULL, but now it is set with out_fence = eb_requests_create() > which returns error pointers. The error path needs to be modified to > avoid an Oops in the "goto err_request;" path. > > Fixes: 544460c33821 ("drm/i915: Multi-BB execbuf") > Signed-off-by: Dan Carpenter Thanks for the fix. LGTM. With that: Reviewed-by: Matthew Brost > --- > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > index 4d7da07442f2..9b24d9b5ade1 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > @@ -3277,6 +3277,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, > out_fence = eb_requests_create(, in_fence, out_fence_fd); > if (IS_ERR(out_fence)) { > err = PTR_ERR(out_fence); > + out_fence = NULL; > if (eb.requests[0]) > goto err_request; > else > -- > 2.20.1 >
Re: [PATCH 2/2] drm/sched: fix dropping the last fence ref
On Tue, Nov 16, 2021 at 10:25:19AM +0100, Christian König wrote: > We need to grab another ref before trying to add the fence to the sched > job and not after. > > Signed-off-by: Christian König Reviewed-by: Daniel Vetter I wondered first why this goes boom, but then I realized that in some cases add_dependency() drops the reference of the passed-in fence. Please also add the Fixes: line like in the previous patch. Cheers, Daniel > --- > drivers/gpu/drm/scheduler/sched_main.c | 10 ++ > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/scheduler/sched_main.c > b/drivers/gpu/drm/scheduler/sched_main.c > index 94fe51b3caa2..400d201c3c28 100644 > --- a/drivers/gpu/drm/scheduler/sched_main.c > +++ b/drivers/gpu/drm/scheduler/sched_main.c > @@ -704,12 +704,14 @@ int drm_sched_job_add_implicit_dependencies(struct > drm_sched_job *job, > int ret; > > dma_resv_for_each_fence(, obj->resv, write, fence) { > - ret = drm_sched_job_add_dependency(job, fence); > - if (ret) > - return ret; > - > /* Make sure to grab an additional ref on the added fence */ > dma_fence_get(fence); > + > + ret = drm_sched_job_add_dependency(job, fence); > + if (ret) { > + dma_fence_put(fence); > + return ret; > + } > } > return 0; > } > -- > 2.25.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 00/15] iio: buffer-dma: write() and new DMABUF based API
On Tue, Nov 16, 2021 at 05:02:25PM +0100, Daniel Vetter wrote: > On Mon, Nov 15, 2021 at 02:57:37PM +, Paul Cercueil wrote: > > Le lun., nov. 15 2021 at 15:37:16 +0100, Daniel Vetter a écrit : > > > On Mon, Nov 15, 2021 at 02:19:10PM +, Paul Cercueil wrote: > > > > Hi Jonathan, > > > > > > > > This patchset introduces a new userspace interface based on DMABUF > > > > objects, to complement the existing fileio based API. > > > > > > > > The advantage of this DMABUF based interface vs. the fileio > > > > interface, is that it avoids an extra copy of the data between the > > > > kernel and userspace. This is particularly userful for high-speed > > > > devices which produce several megabytes or even gigabytes of data per > > > > second. > > > > > > > > The first few patches [01/15] to [03/15] are not really related, but > > > > allow to reduce the size of the patches that introduce the new API. > > > > > > > > Patch [04/15] to [06/15] enables write() support to the buffer-dma > > > > implementation of the buffer API, to continue the work done by > > > > Mihail Chindris. > > > > > > > > Patches [07/15] to [12/15] introduce the new DMABUF based API. > > > > > > > > Patches [13/15] and [14/15] add support for cyclic buffers, only > > > > through > > > > the new API. A cyclic buffer will be repeated on the output until the > > > > buffer is disabled. > > > > > > > > Patch [15/15] adds documentation about the new API. > > > > > > > > For now, the API allows you to alloc DMABUF objects and mmap() them > > > > to > > > > read or write the samples. It does not yet allow to import DMABUFs > > > > parented to other subsystems, but that should eventually be possible > > > > once it's wired. > > > > > > > > This patchset is inspired by the "mmap interface" that was previously > > > > submitted by Alexandru Ardelean and Lars-Peter Clausen, so it would be > > > > great if I could get a review from you guys. Alexandru's commit was > > > > signed with his @analog.com address but he doesn't work at ADI anymore, > > > > so I believe I'll need him to sign with a new email. > > > > > > Why dma-buf? dma-buf looks like something super generic and useful, until > > > you realize that there's a metric ton of gpu/accelerator bagage piled in. > > > So unless buffer sharing with a gpu/video/accel/whatever device is the And cameras (maybe they're included in "whatever" ?). > > > goal here, and it's just for a convenient way to get at buffer handles, > > > this doesn't sound like a good idea. > > > > Good question. The first reason is that a somewhat similar API was intented > > before[1], but refused upstream as it was kind of re-inventing the wheel. > > > > The second reason, is that we want to be able to share buffers too, not with > > gpu/video but with the network (zctap) and in the future with USB > > (functionFS) too. > > > > [1]: > > https://lore.kernel.org/linux-iio/20210217073638.21681-1-alexandru.ardel...@analog.com/T/ > > Hm is that code merged already in upstream already? > > I know that dma-buf looks really generic, but like I said if there's no > need ever to interface with any of the gpu buffer sharing it might be > better to use something else (like get_user_pages maybe, would that work?). Not GUP please. That brings another set of issues, especially when dealing with DMA, we've suffered enough from the USERPTR support in V4L2 to avoid repeating this. Let's make dma-buf better instead. > > > Also if the idea is to this with gpus/accelerators then I'd really like to > > > see the full thing, since most likely at that point you also want > > > dma_fence. And once we talk dma_fence things get truly horrible from a > > > locking pov :-( Or well, just highly constrained and I get to review what > > > iio is doing with these buffers to make sure it all fits. > > > > There is some dma_fence action in patch #10, which is enough for the > > userspace apps to use the API. > > > > What "horribleness" are we talking about here? It doesn't look that scary to > > me, but I certainly don't have the complete picture. > > You need to annotate all the code involved in signalling that dma_fence > using dma_fence_begin/end_signalling, and then enable full lockdep and > everything. > > You can safely assume you'll find bugs, because we even have bugs about > this in gpu drivers (where that annotation isn't fully rolled out yet). > > The tldr is that you can allocate memory in there. And a pile of other > restrictions, but not being able to allocate memory (well GFP_ATOMIC is > ok, but that can fail) is a very serious restriction. > -Daniel > > > > > Alexandru Ardelean (1): > > > >iio: buffer-dma: split iio_dma_buffer_fileio_free() function > > > > > > > > Paul Cercueil (14): > > > >iio: buffer-dma: Get rid of incoming/outgoing queues > > > >iio: buffer-dma: Remove unused iio_buffer_block struct > > > >iio: buffer-dma: Use round_down() instead of rounddown() > >
Re: [PATCH 00/15] iio: buffer-dma: write() and new DMABUF based API
On Mon, Nov 15, 2021 at 02:57:37PM +, Paul Cercueil wrote: > Hi Daniel, > > Le lun., nov. 15 2021 at 15:37:16 +0100, Daniel Vetter a > écrit : > > On Mon, Nov 15, 2021 at 02:19:10PM +, Paul Cercueil wrote: > > > Hi Jonathan, > > > > > > This patchset introduces a new userspace interface based on DMABUF > > > objects, to complement the existing fileio based API. > > > > > > The advantage of this DMABUF based interface vs. the fileio > > > interface, is that it avoids an extra copy of the data between the > > > kernel and userspace. This is particularly userful for high-speed > > > devices which produce several megabytes or even gigabytes of data > > > per > > > second. > > > > > > The first few patches [01/15] to [03/15] are not really related, but > > > allow to reduce the size of the patches that introduce the new API. > > > > > > Patch [04/15] to [06/15] enables write() support to the buffer-dma > > > implementation of the buffer API, to continue the work done by > > > Mihail Chindris. > > > > > > Patches [07/15] to [12/15] introduce the new DMABUF based API. > > > > > > Patches [13/15] and [14/15] add support for cyclic buffers, only > > > through > > > the new API. A cyclic buffer will be repeated on the output until > > > the > > > buffer is disabled. > > > > > > Patch [15/15] adds documentation about the new API. > > > > > > For now, the API allows you to alloc DMABUF objects and mmap() them > > > to > > > read or write the samples. It does not yet allow to import DMABUFs > > > parented to other subsystems, but that should eventually be possible > > > once it's wired. > > > > > > This patchset is inspired by the "mmap interface" that was > > > previously > > > submitted by Alexandru Ardelean and Lars-Peter Clausen, so it would > > > be > > > great if I could get a review from you guys. Alexandru's commit was > > > signed with his @analog.com address but he doesn't work at ADI > > > anymore, > > > so I believe I'll need him to sign with a new email. > > > > Why dma-buf? dma-buf looks like something super generic and useful, > > until > > you realize that there's a metric ton of gpu/accelerator bagage piled > > in. > > So unless buffer sharing with a gpu/video/accel/whatever device is the > > goal here, and it's just for a convenient way to get at buffer handles, > > this doesn't sound like a good idea. > > Good question. The first reason is that a somewhat similar API was intented > before[1], but refused upstream as it was kind of re-inventing the wheel. > > The second reason, is that we want to be able to share buffers too, not with > gpu/video but with the network (zctap) and in the future with USB > (functionFS) too. > > [1]: > https://lore.kernel.org/linux-iio/20210217073638.21681-1-alexandru.ardel...@analog.com/T/ Hm is that code merged already in upstream already? I know that dma-buf looks really generic, but like I said if there's no need ever to interface with any of the gpu buffer sharing it might be better to use something else (like get_user_pages maybe, would that work?). > > Also if the idea is to this with gpus/accelerators then I'd really like > > to > > see the full thing, since most likely at that point you also want > > dma_fence. And once we talk dma_fence things get truly horrible from a > > locking pov :-( Or well, just highly constrained and I get to review > > what > > iio is doing with these buffers to make sure it all fits. > > There is some dma_fence action in patch #10, which is enough for the > userspace apps to use the API. > > What "horribleness" are we talking about here? It doesn't look that scary to > me, but I certainly don't have the complete picture. You need to annotate all the code involved in signalling that dma_fence using dma_fence_begin/end_signalling, and then enable full lockdep and everything. You can safely assume you'll find bugs, because we even have bugs about this in gpu drivers (where that annotation isn't fully rolled out yet). The tldr is that you can allocate memory in there. And a pile of other restrictions, but not being able to allocate memory (well GFP_ATOMIC is ok, but that can fail) is a very serious restriction. -Daniel > > Cheers, > -Paul > > > Cheers, Daniel > > > > > > > > Cheers, > > > -Paul > > > > > > Alexandru Ardelean (1): > > >iio: buffer-dma: split iio_dma_buffer_fileio_free() function > > > > > > Paul Cercueil (14): > > >iio: buffer-dma: Get rid of incoming/outgoing queues > > >iio: buffer-dma: Remove unused iio_buffer_block struct > > >iio: buffer-dma: Use round_down() instead of rounddown() > > >iio: buffer-dma: Enable buffer write support > > >iio: buffer-dmaengine: Support specifying buffer direction > > >iio: buffer-dmaengine: Enable write support > > >iio: core: Add new DMABUF interface infrastructure > > >iio: buffer-dma: Use DMABUFs instead of custom solution > > >iio: buffer-dma: Implement new DMABUF based
Re: [PATCH v4] drm/ttm: Clarify that the TTM_PL_SYSTEM is under TTMs control
> On Nov 16, 2021, at 03:20, Christian König wrote: > > Am 16.11.21 um 08:43 schrieb Thomas Hellström: >> On 11/16/21 08:19, Christian König wrote: >>> Am 13.11.21 um 12:26 schrieb Thomas Hellström: Hi, Zack, On 11/11/21 17:44, Zack Rusin wrote: > On Wed, 2021-11-10 at 09:50 -0500, Zack Rusin wrote: >> TTM takes full control over TTM_PL_SYSTEM placed buffers. This makes >> driver internal usage of TTM_PL_SYSTEM prone to errors because it >> requires the drivers to manually handle all interactions between TTM >> which can swap out those buffers whenever it thinks it's the right >> thing to do and driver. >> >> CPU buffers which need to be fenced and shared with accelerators >> should >> be placed in driver specific placements that can explicitly handle >> CPU/accelerator buffer fencing. >> Currently, apart, from things silently failing nothing is enforcing >> that requirement which means that it's easy for drivers and new >> developers to get this wrong. To avoid the confusion we can document >> this requirement and clarify the solution. >> >> This came up during a discussion on dri-devel: >> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F232f45e9-8748-1243-09bf-56763e6668b3%40amd.comdata=04%7C01%7Czackr%40vmware.com%7C2a4063bb30f84dc921ba08d9a8d9ea8c%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637726476178183950%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=UF0kuamK%2FfETPW22EbEaQ0rUrDqSQbBtFwT5DwknY1s%3Dreserved=0 >> I took a slightly deeper look into this. I think we need to formalize this a bit more to understand pros and cons and what the restrictions are really all about. Anybody looking at the prevous discussion will mostly see arguments similar to "this is stupid and difficult" and "it has always been this way" which are not really constructive. First disregarding all accounting stuff, I think this all boils down to TTM_PL_SYSTEM having three distinct states: 1) POPULATED 2) LIMBO (Or whatever we want to call it. No pages present) 3) SWAPPED. The ttm_bo_move_memcpy() helper understands these, and any standalone driver implementation of the move() callback _currently_ needs to understand these as well, unless using the ttm_bo_move_memcpy() helper. Now using a bounce domain to proxy SYSTEM means that the driver can forget about the SWAPPED state, it's automatically handled by the move setup code. However, another pitfall is LIMBO, in that if when you move from SYSTEM/LIMBO to your bounce domain, the BO will be populated. So any naive accelerated move() implementation creating a 1GB BO in fixed memory, like VRAM, will needlessly allocate and free 1GB of system memory in the process instead of just performing a clear operation. Looks like amdgpu suffers from this? I think what is really needed is either a) A TTM helper that helps move callback implementations resolve the issues populating system from LIMBO or SWAP, and then also formalize driver notification for swapping. At a minimum, I think the swap_notify() callback needs to be able to return a late error. b) Make LIMBO and SWAPPED distinct memory regions. (I think I'd vote for this without looking into it in detail). In both these cases, we should really make SYSTEM bindable by GPU, otherwise we'd just be trading one pitfall for another related without really resolving the root problem. As for fencing not being supported by SYSTEM, I'm not sure why we don't want this, because it would for example prohibit async ttm_move_memcpy(), and also, async unbinding of ttm_tt memory like MOB on vmgfx. (I think it's still sync). There might be an accounting issue related to this as well, but I guess Christian would need to chime in on this. If so, I think it needs to be well understood and documented (in TTM, not in AMD drivers). >>> >>> I think the problem goes deeper than what has been mentioned here so far. >>> >>> Having fences attached to BOs in the system domain is probably ok, but the >>> key point is that the BOs in the system domain are under TTMs control and >>> should not be touched by the driver. >>> >>> What we have now is that TTMs internals like the allocation state of BOs in >>> system memory (the populated, limbo, swapped you mentioned above) is >>> leaking into the drivers and I think exactly that is the part which doesn't >>> work reliable here. You can of course can get that working, but that >>> requires knowledge of the internal state which in my eyes was always >>> illegal. >>> >> Well, I tend to agree to some extent, but then, like said above even >>
[PATCH v3] drm/i915/rpm: Enable runtime pm autosuspend by default
v1: Enable runtime pm autosuspend by default for Gen12 and later versions. v2: Enable runtime pm autosuspend by default for all platforms(Syrjala Ville) v3: Change commit message(Nikula Jani) Let's enable runtime pm autosuspend by default everywhere. So, we can allow D3hot and bigger power savings on idle scenarios. But at this time let's not touch the autosuspend_delay time, what caused some regression on our previous attempt. Also, the latest identified issue on GuC PM has been fixed by commit 1a52faed3131 ("drm/i915/guc: Take GT PM ref when deregistering context") Signed-off-by: Tilak Tangudu --- drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 0d85f3c5c526..22dab36afcb6 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -590,6 +590,9 @@ void intel_runtime_pm_enable(struct intel_runtime_pm *rpm) pm_runtime_use_autosuspend(kdev); } + /* Enable by default */ + pm_runtime_allow(kdev); + /* * The core calls the driver load handler with an RPM reference held. * We drop that here and will reacquire it during unloading in -- 2.25.1
[PATCH] drm/scheduler: fix drm_sched_job_add_implicit_dependencies harder
From: Rob Clark drm_sched_job_add_dependency() could drop the last ref, so we need to do the dma_fence_get() first. Cc: Christian König Fixes: 9c2ba265352a drm/scheduler: ("use new iterator in drm_sched_job_add_implicit_dependencies v2") Signed-off-by: Rob Clark --- Applies on top of "drm/scheduler: fix drm_sched_job_add_implicit_dependencies" but I don't think that has a stable commit sha yet. drivers/gpu/drm/scheduler/sched_main.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 94fe51b3caa2..f91fb31ab7a7 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -704,12 +704,13 @@ int drm_sched_job_add_implicit_dependencies(struct drm_sched_job *job, int ret; dma_resv_for_each_fence(, obj->resv, write, fence) { - ret = drm_sched_job_add_dependency(job, fence); - if (ret) - return ret; - /* Make sure to grab an additional ref on the added fence */ dma_fence_get(fence); + ret = drm_sched_job_add_dependency(job, fence); + if (ret) { + dma_fence_put(fence); + return ret; + } } return 0; } -- 2.33.1
Re: [PATCH v4] drm/ttm: Clarify that the TTM_PL_SYSTEM is under TTMs control
On Tue, Nov 16, 2021 at 10:09:44AM +0100, Christian König wrote: > Am 16.11.21 um 10:00 schrieb Thomas Hellström: > > On 11/16/21 09:54, Christian König wrote: > > > Am 16.11.21 um 09:33 schrieb Thomas Hellström: > > > > On 11/16/21 09:20, Christian König wrote: > > > > > Am 16.11.21 um 08:43 schrieb Thomas Hellström: > > > > > > On 11/16/21 08:19, Christian König wrote: > > > > > > > [SNIP] > > > > > > > > > > Well my long term plan is to audit the code base once more > > > > > and remove the limbo state from the SYSTEM domain. > > > > > > > > > > E.g. instead of a SYSTEM BO without pages you allocate a BO > > > > > without a resource in general which is now possible since > > > > > bo->resource is a pointer. > > > > > > > > > > This would still allow us to allocate "empty shell" BOs. But > > > > > a validation of those BOs doesn't cause a move, but rather > > > > > just allocates the resource for the first time. > > > > > > > > > > The problem so far was just that we access bo->resource way > > > > > to often without checking it. > > > > > > > > So the driver would then at least need to be aware of these > > > > empty shell bos without resource for their move callbacks? > > > > (Again thinking of the move from empty shell -> VRAM). > > > > > > My thinking goes more into the direction that this looks like a BO > > > directly allocated in VRAM to the driver. > > > > > > We could of course also make it a move, but of hand I don't see a > > > reason for it. > > > > As long as there is a way to provide accelerated VRAM clearing if > > necessary the directly allocated view sounds fine with me. (Looking at > > amdgpu it looks like you clear on resource destroy? I'm not fully sure > > that would work with all i915 use cases) > > In amdgpu we have both. The AMDGPU_GEM_CREATE_VRAM_CLEARED flag clears the > memory on allocation and AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE flag makes > sure it is wiped on release. btw how does this work when there was random previous stuff in that memory region previously? Like the more we do hmm style integration the more you cannot assume that critical memory is cleared on free/release, because that's just not how core mm/ works, where pages are all cleared on alloc before userspace sees them. Entirely sidetrack discussion :-) -Daniel > Wiping on release is sometimes faster because you don't need to wait for the > clear to finish before you can first use it. > > But thinking about it once more it might be a good idea to have move > callbacks for empty shells and freshly allocated BOs as well, so that the > driver is informed about the state change of the BO. > > Regards, > Christian. > > > > > /Thomas > > > > > > > > > > Christian. > > > > > > > > > > > Thanks, > > > > > > > > /Thomas > > > > > > > > > > > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v3 11/12] drm/virtio: implement context init: add virtio_gpu_fence_event
On Mon, Nov 15, 2021 at 07:26:14PM +, Kasireddy, Vivek wrote: > Hi Daniel, Greg, > > If it is the same or a similar crash reported here: > https://lists.freedesktop.org/archives/dri-devel/2021-November/330018.html > and here: > https://lists.freedesktop.org/archives/dri-devel/2021-November/330212.html > then the fix is already merged: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d89c0c8322ecdc9a2ec84b959b6f766be082da76 Yeah but that still leaves the problem of why exaxtly virtgpu is reinventing drm_poll here? Can you please replace it with drm_poll like all other drivers, including the ones that have private events? Thanks, Daniel > > Thanks, > Vivek > > > On Sat, Nov 13, 2021 at 03:51:48PM +0100, Greg KH wrote: > > > On Tue, Sep 21, 2021 at 04:20:23PM -0700, Gurchetan Singh wrote: > > > > Similar to DRM_VMW_EVENT_FENCE_SIGNALED. Sends a pollable event > > > > to the DRM file descriptor when a fence on a specific ring is > > > > signaled. > > > > > > > > One difference is the event is not exposed via the UAPI -- this is > > > > because host responses are on a shared memory buffer of type > > > > BLOB_MEM_GUEST [this is the common way to receive responses with > > > > virtgpu]. As such, there is no context specific read(..) > > > > implementation either -- just a poll(..) implementation. > > > > > > > > Signed-off-by: Gurchetan Singh > > > > Acked-by: Nicholas Verne > > > > --- > > > > drivers/gpu/drm/virtio/virtgpu_drv.c | 43 +- > > > > drivers/gpu/drm/virtio/virtgpu_drv.h | 7 + > > > > drivers/gpu/drm/virtio/virtgpu_fence.c | 10 ++ > > > > drivers/gpu/drm/virtio/virtgpu_ioctl.c | 34 > > > > 4 files changed, 93 insertions(+), 1 deletion(-) > > > > > > This commit seems to cause a crash in a virtual drm gpu driver for > > > Android. I have reverted this, and the next commit in the series from > > > Linus's tree and all is good again. > > > > > > Any ideas? > > > > Well no, but also this patch looks very questionable of hand-rolling > > drm_poll. Yes you can do driver private events like > > DRM_VMW_EVENT_FENCE_SIGNALED, that's fine. But you really should not need > > to hand-roll the poll callback. vmwgfx (which generally is a very old > > driver which has lots of custom stuff, so not a great example) doesn't do > > that either. > > > > So that part should go no matter what I think. > > -Daniel > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v3 0/9] backlight: qcom-wled: fix and solidify handling of enabled-strings
On Tue, 16 Nov 2021, Daniel Thompson wrote: > Hi Lee > > On Mon, Nov 15, 2021 at 09:34:50PM +0100, Marijn Suijten wrote: > > This patchset fixes WLED's handling of enabled-strings: besides some > > cleanup it is now actually possible to specify a non-contiguous array of > > enabled strings (not necessarily starting at zero) and the values from > > DT are now validated to prevent possible unexpected out-of-bounds > > register and array element accesses. > > Off-by-one mistakes in the maximum number of strings, also causing > > out-of-bounds access, have been addressed as well. > > They have arrived piecemeal (during v1, v2 and v3) but all patches on > the set should now have my R-b: attached to them. I can see that. Nothing for you to worry about. I'll apply these when I conduct my next sweep, thanks. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH] omapfb: add one more "fallthrough" statement
On Tue, Nov 16, 2021 at 10:21:54AM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann > > clang warns about one missing fallthrough that gcc ignores: > > drivers/video/fbdev/omap/omapfb_main.c:1558:2: error: unannotated > fall-through between switch labels [-Werror,-Wimplicit-fallthrough] > > Add it here as well. > > Signed-off-by: Arnd Bergmann Reviewed-by: Nathan Chancellor > --- > drivers/video/fbdev/omap/omapfb_main.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/video/fbdev/omap/omapfb_main.c > b/drivers/video/fbdev/omap/omapfb_main.c > index 3d090d2d9ed9..a6a8404d1462 100644 > --- a/drivers/video/fbdev/omap/omapfb_main.c > +++ b/drivers/video/fbdev/omap/omapfb_main.c > @@ -1555,6 +1555,7 @@ static void omapfb_free_resources(struct omapfb_device > *fbdev, int state) > case 1: > dev_set_drvdata(fbdev->dev, NULL); > kfree(fbdev); > + fallthrough; > case 0: > /* nothing to free */ > break; > -- > 2.29.2 >
Re: [Intel-gfx] [PATCH] drm/i915/guc: fix NULL vs IS_ERR() checking
On Tue, Nov 16, 2021 at 02:49:16PM +0300, Dan Carpenter wrote: > The intel_engine_create_virtual() function does not return NULL. It > returns error pointers. > > Fixes: e5e32171a2cf ("drm/i915/guc: Connect UAPI to GuC multi-lrc interface") > Signed-off-by: Dan Carpenter Reviewed-by: Rodrigo Vivi > --- > drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > index 38b47e73e35d..c48557dfa04c 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > @@ -3080,8 +3080,8 @@ guc_create_parallel(struct intel_engine_cs **engines, > > ce = intel_engine_create_virtual(siblings, num_siblings, >FORCE_VIRTUAL); > - if (!ce) { > - err = ERR_PTR(-ENOMEM); > + if (IS_ERR(ce)) { > + err = ERR_CAST(ce); > goto unwind; > } > > -- > 2.20.1 >
Re: [PATCH v6 7/7] drm/mediatek: Add mt8195 DisplayPort driver
Hi, On Mon, Nov 15, 2021 at 09:33:52AM -0500, Guillaume Ranquet wrote: > Quoting Maxime Ripard (2021-11-15 11:11:29) > > > The driver creates a child device for the phy. The child device will > > > never exist without the parent being active. As they are sharing a > > > register range, the parent passes a regmap pointer to the child so that > > > both can work with the same register range. The phy driver sets device > > > data that is read by the parent to get the phy device that can be used > > > to control the phy properties. > > > > If the PHY is in the same register space than the DP controller, why do > > you need a separate PHY driver in the first place? > > This has been asked by Chun-Kuang Hu in a previous revision of the series: > > https://lore.kernel.org/linux-mediatek/CAAOTY_-+T-wRCH2yw2XSm=zbabbqbq4eqpu2p0tf90gawqe...@mail.gmail.com/ It's a bit of a circular argument though :) It's a separate phy driver because it needs to go through another maintainer's tree, but it needs to go through another maintainer's tree because it's a separate phy driver. It doesn't explain why it needs to be a separate phy driver? Why can't the phy setup be done directly in the DP driver, if it's essentially a single device? That being said, usually what those kind of questions mean is that you're missing a comment or something in the commit log to provide that context in the first place, so it would be great to add that context here. And it will avoid the situation we're now in where multiple reviewers ask the same questions over and over again :) > > > +static void mtk_dp_bridge_atomic_enable(struct drm_bridge *bridge, > > > + struct drm_bridge_state *old_state) > > > +{ > > > + struct mtk_dp *mtk_dp = mtk_dp_from_bridge(bridge); > > > + struct drm_connector *conn; > > > + struct drm_connector_state *conn_state; > > > + struct drm_crtc *crtc; > > > + struct drm_crtc_state *crtc_state; > > > + int ret = 0; > > > + int i; > > > + > > > + conn = > > > drm_atomic_get_new_connector_for_encoder(old_state->base.state, > > > + bridge->encoder); > > > + if (!conn) { > > > + drm_err(mtk_dp->drm_dev, > > > + "Can't enable bridge as connector is missing\n"); > > > + return; > > > + } > > > + > > > + memcpy(mtk_dp->connector_eld, conn->eld, MAX_ELD_BYTES); > > > > This should be protected by a mutex (just like any resource shared > > between KMS and ALSA) > > Ok. I forgot to ask (even though checkpatch does mention it iirc), but since you have multiple mutex it would be nice to have a comment for each mutex stating exactly what it protects, and against what. It's hard otherwise to remember or figure out what the locks are here for. > > > + ret = mtk_dp_dt_parse_pdata(mtk_dp, pdev); > > > + if (ret) > > > + return ret; > > > > pdata? > > > can you elaborate? Sorry, yeah, pdata is usually the abbreviation used in linux for the platform_data mechanism, but you're using the DT to retrieve your resources (and platform_data usually don't involve any parsing), so the name is odd. > > > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c > > > b/drivers/gpu/drm/mediatek/mtk_dpi.c > > > index 384074f69111b..e6e88e3cd811d 100644 > > > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c > > > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c > > > @@ -63,6 +63,14 @@ enum mtk_dpi_out_color_format { > > > MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL > > > }; > > > > > > +enum TVDPLL_CLK { > > > + TVDPLL_PLL = 0, > > > + TVDPLL_D2 = 2, > > > + TVDPLL_D4 = 4, > > > + TVDPLL_D8 = 8, > > > + TVDPLL_D16 = 16, > > > +}; > > > + > > > struct mtk_dpi { > > > struct drm_encoder encoder; > > > struct drm_bridge bridge; > > > @@ -71,8 +79,10 @@ struct mtk_dpi { > > > void __iomem *regs; > > > struct device *dev; > > > struct clk *engine_clk; > > > + struct clk *dpi_ck_cg; > > > struct clk *pixel_clk; > > > struct clk *tvd_clk; > > > + struct clk *pclk_src[5]; > > > int irq; > > > struct drm_display_mode mode; > > > const struct mtk_dpi_conf *conf; > > > @@ -135,6 +145,7 @@ struct mtk_dpi_conf { > > > u32 hvsize_mask; > > > u32 channel_swap_shift; > > > u32 yuv422_en_bit; > > > + u32 csc_enable_bit; > > > const struct mtk_dpi_yc_limit *limit; > > > }; > > > > > > @@ -365,7 +376,7 @@ static void mtk_dpi_config_yuv422_enable(struct > > > mtk_dpi *dpi, bool enable) > > > > > > static void mtk_dpi_config_csc_enable(struct mtk_dpi *dpi, bool enable) > > > { > > > - mtk_dpi_mask(dpi, DPI_CON, enable ? CSC_ENABLE : 0, CSC_ENABLE); > > > + mtk_dpi_mask(dpi, DPI_CON, enable ? dpi->conf->csc_enable_bit : 0, > > > dpi->conf->csc_enable_bit); > > > } > > > > > > static void mtk_dpi_config_swap_input(struct mtk_dpi *dpi, bool enable) > > > @@ -384,22
Re: Questions about KMS flip
On 2021-11-16 15:10, Alex Deucher wrote: > On Tue, Nov 16, 2021 at 3:09 AM Christian König > wrote: >> >> Am 16.11.21 um 09:00 schrieb Lang Yu: >>> On Tue, Nov 16, 2021 at 08:14:08AM +0100, Christian KKKnig wrote: Am 16.11.21 um 04:27 schrieb Lang Yu: > On Mon, Nov 15, 2021 at 01:04:15PM +0100, Michel DDDnzer wrote: >> [SNIP] >>> Though a single call to dce_v*_0_crtc_do_set_base() will >>> only pin the BO, I found it will be unpinned in next call to >>> dce_v*_0_crtc_do_set_base(). >> Yeah, that's the normal case when the new BO is different from the old >> one. >> >> To catch the case I described, try something like >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c >> b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c >> >> index 18a7b3bd633b..5726bd87a355 100644 >> >> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c >> >> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c >> >> @@ -1926,6 +1926,7 @@ static int dce_v11_0_crtc_do_set_base(struct >> drm_crtc *crtc, >> >> return r; >> >> >> >> if (!atomic) { >> >> + WARN_ON_ONCE(target_fb == fb); >> >> r = amdgpu_bo_pin(abo, AMDGPU_GEM_DOMAIN_VRAM); >> >> if (unlikely(r != 0)) { >> >> amdgpu_bo_unreserve(abo); >> > I did some tests, the warning can be triggered. > > pin/unpin operations in *_crtc_do_set_base() and > amdgpu_display_crtc_page_flip_target() are mixed. Ok sounds like we narrowed down the root cause pretty well. Question is now how can we fix this? Just not pin the BO when target_fb == fb? >>> That worked. I did a few simple tests and didn't observe ttm_bo_release >>> warnings >>> any more. >>> >>> The pin/unpin logic, >>> >>> 1, fist crtc_mode_set, dce_v*_0_crtc_do_set_base() pins >>> crtc->primary->fb(new), >>> old_fb is NULL. >>> >>> 2, second crtc_mode_set, dce_v*_0_crtc_do_set_base() pins >>> crtc->primary->fb(new), >>> unpins old fb. >>> >>> 3, amdgpu_display_crtc_page_flip_target() pin/unpin operations. >>> >>> 4, third crtc_mode_set, dce_v*_0_crtc_do_set_base() pins >>> crtc->primary->fb(new), >>> unpins old fb (it is pinned in last call to >>> amdgpu_display_crtc_page_flip_target) >>> >>> 5, amdgpu_display_crtc_page_flip_target() pin/unpin operations. >>> >>> . >>> >>> x, reboot, amdgpu_display_suspend_helper() is called, the last pinned fb >>> was unpinned. >>> >>> And I didn't observe amdgpu_bo_unpin() in dce_v*_0_crtc_disable() is called. >>> >>> If the logic is wrong, please correct me. >> >> I can't fully judge because I'm not that deep with my head in the old >> display code, but from a ten mile high view it sounds sane to me. Michel >> what do you think? Sounds good to me. Would be nice to address the other issue identified in this thread as well: The pin in amdgpu_display_crtc_page_flip_target is guarded by if (!adev->enable_virtual_display), but the corresponding unpins in amdgpu_display_unpin_work_func & dce_v*_0_crtc_disable aren't. This probably results in pin count underflows with virtual display. -- Earthling Michel Dänzer| https://redhat.com Libre software enthusiast | Mesa and Xwayland developer
Re: [Intel-gfx] [PATCH v2 1/1] drm/i915/rpm: Enable runtime pm autosuspend by default
On Mon, Nov 15, 2021 at 09:58:56AM -0500, Rodrigo Vivi wrote: > On Mon, Nov 15, 2021 at 01:44:57PM +0200, Jani Nikula wrote: > > On Mon, 15 Nov 2021, Tilak Tangudu wrote: > > > > The actual commit message with explanations why it will work now and > > didn't work before go here. > > The truth is that: > > 1. We don't have a good track of *all* the issues with the past attempts. > 2. But apparently in every attempt we hit some other bug, like the latest >one with GuC PM... > 3. All the attempts we also tried to do multiple changes at the same time, >including reducing the autosuspend delay. > > > Just the changelog will not be enough. > > But yes, you are absolutely right here. changelogs are not enough and > we need some explanation in the commit itself. > > I'd suggest something like: > > """ > Let's enable runtime pm autosuspend by default everywhere. So we can > allow D3hot and bigger power savings on idle scenarios. > > But at this time let's not touch the autosuspend_delay time, > what caused some regression on our previous attempt. > > Also, the latest identified issue on GuC PM has been fixed by > 1a52faed3131 ("drm/i915/guc: Take GT PM ref when deregistering context") > > """ > > While writing that I remembered that we cannot do this just yet. > We need to do a further work and block the d3cold on discrete. > D3cold is not ready yet and enabling this autosuspend by default > will blow up some discrete experimental usages of upstream i915 > out there. Let's protect that first. we now have the d3cold in place. Please proceed with the spin on the patch with the commit message improvement. Thanks, Rodrigo. > > Thanks, > Rodrigo. > > > > > BR, > > Jani. > > > > > > > v1: Enable runtime pm autosuspend by default for Gen12 > > > and later versions. > > > > > > v2: Enable runtime pm autosuspend by default for all > > > platforms(Syrjala Ville) > > > > > > Signed-off-by: Tilak Tangudu > > > --- > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > index eaf7688f517d..f26ed1427fdc 100644 > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > @@ -600,6 +600,9 @@ void intel_runtime_pm_enable(struct intel_runtime_pm > > > *rpm) > > > pm_runtime_use_autosuspend(kdev); > > > } > > > > > > + /* Enable by default */ > > > + pm_runtime_allow(kdev); > > > + > > > /* > > >* The core calls the driver load handler with an RPM reference held. > > >* We drop that here and will reacquire it during unloading in > > > > -- > > Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH] drm/i915: Disable D3Cold in s2idle and runtime pm
On Mon, Nov 15, 2021 at 02:20:36PM -0500, Rodrigo Vivi wrote: > On Mon, Nov 15, 2021 at 09:10:54PM +0530, Tilak Tangudu wrote: > > s2idle and runtime pm puts the pci gfx device in D3Hot, ACPI runtime > > monitors the pci tree,if it sees complete tree as D3Hot,it transitions > > the device to D3Cold.But i915 do not have D3Cold support in S2idle or in > > runtime pm. so disabling D3cold in above flows and its FIXME. > > > > Added pci D3Cold enable/disable in s2idle and runtime suspend/resume > > flows. > > > > Signed-off-by: Tilak Tangudu > > Just for the clear record, I always preferred the unconditional disallow > of d3cold, but it looks some internal experiments for s3/s4 failed with that > and this approach here was the safest one, so let's move with this and > prevent the d3cold for now and then allow the runtime_pm autosuspend > enabled by default everywhere. > > Reviewed-by: Rodrigo Vivi and pushed, thanks for the patch. > > > --- > > drivers/gpu/drm/i915/i915_drv.c | 19 +++ > > 1 file changed, 19 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > b/drivers/gpu/drm/i915/i915_drv.c > > index 46bf3315f616..af6868f12ef0 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -1194,6 +1194,14 @@ static int i915_drm_suspend_late(struct drm_device > > *dev, bool hibernation) > > goto out; > > } > > > > + /* > > +* FIXME: Temporary hammer to avoid freezing the machine on our DGFX > > +* This should be totally removed when we handle the pci states properly > > +* on runtime PM and on s2idle cases. > > +*/ > > + if (suspend_to_idle(dev_priv)) > > + pci_d3cold_disable(pdev); > > + > > pci_disable_device(pdev); > > /* > > * During hibernation on some platforms the BIOS may try to access > > @@ -1357,6 +1365,8 @@ static int i915_drm_resume_early(struct drm_device > > *dev) > > > > pci_set_master(pdev); > > > > + pci_d3cold_enable(pdev); > > + > > disable_rpm_wakeref_asserts(_priv->runtime_pm); > > > > ret = vlv_resume_prepare(dev_priv, false); > > @@ -1533,6 +1543,7 @@ static int intel_runtime_suspend(struct device *kdev) > > { > > struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > > struct intel_runtime_pm *rpm = _priv->runtime_pm; > > + struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev); > > int ret; > > > > if (drm_WARN_ON_ONCE(_priv->drm, !HAS_RUNTIME_PM(dev_priv))) > > @@ -1578,6 +1589,12 @@ static int intel_runtime_suspend(struct device *kdev) > > drm_err(_priv->drm, > > "Unclaimed access detected prior to suspending\n"); > > > > + /* > > +* FIXME: Temporary hammer to avoid freezing the machine on our DGFX > > +* This should be totally removed when we handle the pci states properly > > +* on runtime PM and on s2idle cases. > > +*/ > > + pci_d3cold_disable(pdev); > > rpm->suspended = true; > > > > /* > > @@ -1616,6 +1633,7 @@ static int intel_runtime_resume(struct device *kdev) > > { > > struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > > struct intel_runtime_pm *rpm = _priv->runtime_pm; > > + struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev); > > int ret; > > > > if (drm_WARN_ON_ONCE(_priv->drm, !HAS_RUNTIME_PM(dev_priv))) > > @@ -1628,6 +1646,7 @@ static int intel_runtime_resume(struct device *kdev) > > > > intel_opregion_notify_adapter(dev_priv, PCI_D0); > > rpm->suspended = false; > > + pci_d3cold_enable(pdev); > > if (intel_uncore_unclaimed_mmio(_priv->uncore)) > > drm_dbg(_priv->drm, > > "Unclaimed access during suspend, bios?\n"); > > -- > > 2.25.1 > >
[PATCH 1/2] dt-bindings: display: Turn lvds.yaml into a generic schema
The lvds.yaml file so far was both defining the generic LVDS properties (such as data-mapping) that could be used for any LVDS sink, but also the panel-lvds binding. That last binding was to describe LVDS panels simple enough, and had a number of other bindings using it as a base to specialise it further. However, this situation makes it fairly hard to extend and reuse both the generic parts, and the panel-lvds itself. Let's remove the panel-lvds parts and leave only the generic LVDS properties. Signed-off-by: Maxime Ripard --- .../display/panel/advantech,idk-1110wr.yaml | 17 ++- .../display/panel/innolux,ee101ia-01d.yaml| 21 +- .../bindings/display/panel/lvds.yaml | 29 +-- .../display/panel/mitsubishi,aa104xd12.yaml | 17 ++- .../display/panel/mitsubishi,aa121td01.yaml | 17 ++- .../display/panel/sgd,gktw70sdae4se.yaml | 17 ++- 6 files changed, 85 insertions(+), 33 deletions(-) diff --git a/Documentation/devicetree/bindings/display/panel/advantech,idk-1110wr.yaml b/Documentation/devicetree/bindings/display/panel/advantech,idk-1110wr.yaml index 93878c2cd370..f27cd2038636 100644 --- a/Documentation/devicetree/bindings/display/panel/advantech,idk-1110wr.yaml +++ b/Documentation/devicetree/bindings/display/panel/advantech,idk-1110wr.yaml @@ -11,13 +11,23 @@ maintainers: - Thierry Reding allOf: + - $ref: panel-common.yaml# - $ref: lvds.yaml# +select: + properties: +compatible: + contains: +const: advantech,idk-1110wr + + required: +- compatible + properties: compatible: items: - const: advantech,idk-1110wr - - {} # panel-lvds, but not listed here to avoid false select + - const: panel-lvds data-mapping: const: jeida-24 @@ -35,6 +45,11 @@ additionalProperties: false required: - compatible + - data-mapping + - width-mm + - height-mm + - panel-timing + - port examples: - |+ diff --git a/Documentation/devicetree/bindings/display/panel/innolux,ee101ia-01d.yaml b/Documentation/devicetree/bindings/display/panel/innolux,ee101ia-01d.yaml index a69681e724cb..6e06eecac14e 100644 --- a/Documentation/devicetree/bindings/display/panel/innolux,ee101ia-01d.yaml +++ b/Documentation/devicetree/bindings/display/panel/innolux,ee101ia-01d.yaml @@ -11,15 +11,26 @@ maintainers: - Thierry Reding allOf: + - $ref: panel-common.yaml# - $ref: lvds.yaml# +select: + properties: +compatible: + contains: +const: innolux,ee101ia-01d + + required: +- compatible + properties: compatible: items: - const: innolux,ee101ia-01d - - {} # panel-lvds, but not listed here to avoid false select + - const: panel-lvds backlight: true + data-mapping: true enable-gpios: true power-supply: true width-mm: true @@ -27,5 +38,13 @@ properties: panel-timing: true port: true +required: + - compatible + - data-mapping + - width-mm + - height-mm + - panel-timing + - port + additionalProperties: false ... diff --git a/Documentation/devicetree/bindings/display/panel/lvds.yaml b/Documentation/devicetree/bindings/display/panel/lvds.yaml index 49460c9dceea..5281a75c8bb5 100644 --- a/Documentation/devicetree/bindings/display/panel/lvds.yaml +++ b/Documentation/devicetree/bindings/display/panel/lvds.yaml @@ -4,7 +4,7 @@ $id: http://devicetree.org/schemas/display/panel/lvds.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# -title: LVDS Display Panel +title: LVDS Display Common Properties maintainers: - Laurent Pinchart @@ -26,18 +26,7 @@ description: |+ Device compatible with those specifications have been marketed under the FPD-Link and FlatLink brands. -allOf: - - $ref: panel-common.yaml# - properties: - compatible: -contains: - const: panel-lvds -description: - Shall contain "panel-lvds" in addition to a mandatory panel-specific - compatible string defined in individual panel bindings. The "panel-lvds" - value shall never be used on its own. - data-mapping: enum: - jeida-18 @@ -96,22 +85,6 @@ properties: If set, reverse the bit order described in the data mappings below on all data lanes, transmitting bits for slots 6 to 0 instead of 0 to 6. - port: true - ports: true - -required: - - compatible - - data-mapping - - width-mm - - height-mm - - panel-timing - -oneOf: - - required: - - port - - required: - - ports - additionalProperties: true ... diff --git a/Documentation/devicetree/bindings/display/panel/mitsubishi,aa104xd12.yaml b/Documentation/devicetree/bindings/display/panel/mitsubishi,aa104xd12.yaml index b5e7ee230fa6..e684b9771532 100644 --- a/Documentation/devicetree/bindings/display/panel/mitsubishi,aa104xd12.yaml +++ b/Documentation/devicetree/bindings/display/panel/mitsubishi,aa104xd12.yaml @@ -11,13 +11,23 @@ maintainers: - Thierry Reding allOf: + - $ref:
[PATCH 2/2] dt-bindings: panel: Introduce a panel-lvds binding
Following the previous patch, let's introduce a generic panel-lvds binding that documents the panels that don't have any particular constraint documented. Signed-off-by: Maxime Ripard --- .../bindings/display/panel/panel-lvds.yaml| 56 +++ 1 file changed, 56 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/panel-lvds.yaml diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml b/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml new file mode 100644 index ..f6ce8e29391e --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml @@ -0,0 +1,56 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/panel/panel-lvds.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Generic LVDS Display Panel Device Tree Bindings + +maintainers: + - Lad Prabhakar + - Thierry Reding + +allOf: + - $ref: panel-common.yaml# + - $ref: lvds.yaml# + +select: + properties: +compatible: + contains: +const: panel-lvds + + not: +properties: + compatible: +contains: + enum: + - advantech,idk-1110wr + - innolux,ee101ia-01d + - mitsubishi,aa104xd12 + - mitsubishi,aa121td01 + - sgd,gktw70sdae4se + + required: +- compatible + +properties: + compatible: +items: + - enum: + - auo,b101ew05 + - tbs,a711-panel + + - const: panel-lvds + +unevaluatedProperties: false + +required: + - compatible + - data-mapping + - width-mm + - height-mm + - panel-timing + - port + +... -- 2.33.1
Re: Questions about KMS flip
On Tue, Nov 16, 2021 at 3:09 AM Christian König wrote: > > Am 16.11.21 um 09:00 schrieb Lang Yu: > > On Tue, Nov 16, 2021 at 08:14:08AM +0100, Christian KKKnig wrote: > >> Am 16.11.21 um 04:27 schrieb Lang Yu: > >>> On Mon, Nov 15, 2021 at 01:04:15PM +0100, Michel DDDnzer wrote: > [SNIP] > > Though a single call to dce_v*_0_crtc_do_set_base() will > > only pin the BO, I found it will be unpinned in next call to > > dce_v*_0_crtc_do_set_base(). > Yeah, that's the normal case when the new BO is different from the old > one. > > To catch the case I described, try something like > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > > index 18a7b3bd633b..5726bd87a355 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > > @@ -1926,6 +1926,7 @@ static int dce_v11_0_crtc_do_set_base(struct > drm_crtc *crtc, > > return r; > > > > if (!atomic) { > > + WARN_ON_ONCE(target_fb == fb); > > r = amdgpu_bo_pin(abo, AMDGPU_GEM_DOMAIN_VRAM); > > if (unlikely(r != 0)) { > > amdgpu_bo_unreserve(abo); > > >>> I did some tests, the warning can be triggered. > >>> > >>> pin/unpin operations in *_crtc_do_set_base() and > >>> amdgpu_display_crtc_page_flip_target() are mixed. > >> Ok sounds like we narrowed down the root cause pretty well. > >> > >> Question is now how can we fix this? Just not pin the BO when target_fb == > >> fb? > > That worked. I did a few simple tests and didn't observe ttm_bo_release > > warnings > > any more. > > > > The pin/unpin logic, > > > > 1, fist crtc_mode_set, dce_v*_0_crtc_do_set_base() pins > > crtc->primary->fb(new), > > old_fb is NULL. > > > > 2, second crtc_mode_set, dce_v*_0_crtc_do_set_base() pins > > crtc->primary->fb(new), > > unpins old fb. > > > > 3, amdgpu_display_crtc_page_flip_target() pin/unpin operations. > > > > 4, third crtc_mode_set, dce_v*_0_crtc_do_set_base() pins > > crtc->primary->fb(new), > > unpins old fb (it is pinned in last call to > > amdgpu_display_crtc_page_flip_target) > > > > 5, amdgpu_display_crtc_page_flip_target() pin/unpin operations. > > > > . > > > > x, reboot, amdgpu_display_suspend_helper() is called, the last pinned fb > > was unpinned. > > > > And I didn't observe amdgpu_bo_unpin() in dce_v*_0_crtc_disable() is called. > > > > If the logic is wrong, please correct me. > > I can't fully judge because I'm not that deep with my head in the old > display code, but from a ten mile high view it sounds sane to me. Michel > what do you think? > > BTW: Nicholas are there any plans to get rid of all that stuff? It would > be a really nice cleanup of rather flaky code I think. We just need to add analog support to the DC code. Darren was looking into it. Alex > > Thanks, > Christian. > > > > > Regards, > > Lang > > > >> Thanks, > >> Christian. > >> > >>> Regards, > >>> Lang > >>> >
Re: [PATCH v2] fbdev: Prevent probing generic drivers if a FB is already registered
On 11/16/21 11:01, Javier Martinez Canillas wrote: > Hello Geert, > > On 11/16/21 10:43, Geert Uytterhoeven wrote: > > [snip] > >>> >>> So this is already a fragile solution and $SUBJECT doesn't make things worse >>> IMO. Since not having something like this can lead to issues as reported by: >>> >>> https://lore.kernel.org/all/2020200253.rfudkt3edbd3nsyj@lahvuun/ >>> >>> We could probably do some smarter here by providing a function that checks >>> if the registered fbdev drivers matches the aperture base. But I'm unsure >>> if that's worth it. After all, fbdev drivers are likely to be disabled by >>> most distros soon now that we have the simpledrm driver. >> >> Checking the aperture base is what was done in all other cases of >> preventing generic (fbdev) drivers from stepping on specific drivers' >> toes... >> > > Ok, I can re-spin the patch checking if the aperture ranges overlap. There's > an apertures_overlap() function in drivers/video/fbdev/core/fbmem.c that can > be exported for fbdev drivers to use. > So I tried the following patch [0]. But when testing on a VM, the efifb driver is probed even after the virtio_gpu driver has already been probed. Being a DRM driver, it doesn't use the fbdev infra and AFAIU doesn't reserve any apertures. When the {efi,simple}fb drivers check if there's an aperture already reserved using the fb_aperture_registered() helper, this just returns false even when a driver for the same hardware was already registered. The kernel log says: [0.891512] checking generic (0 0) vs hw (c000 1d5000) That is because when DRM_FBDEV_EMULATION=y, the virtio_gpu driver registers an fbdev but without any aperture set. I discussed this with Thomas and even though $SUBJECT is just a workaround, it seems that is the best we can do as an heuristic to prevent the generic fbdev drivers to be probed after a native DRM driver. [0]: >From d962c20bc9fd90c2525d79b69e632d99e8050fc5 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Thu, 11 Nov 2021 00:55:06 +0100 Subject: [PATCH v4] fbdev: Prevent probing generic drivers if a FB is already registered The efifb and simplefb drivers just render to a pre-allocated frame buffer and rely on the display hardware being initialized before the kernel boots. But if another driver already probed correctly and registered a fbdev, the generic drivers shouldn't be probed since an actual driver for the display hardware is already present. This is more likely to occur after commit d391c5827107 ("drivers/firmware: move x86 Generic System Framebuffers support") since the "efi-framebuffer" and "simple-framebuffer" platform devices are registered at a later time. Link: https://lore.kernel.org/r/2020200253.rfudkt3edbd3nsyj@lahvuun/ Fixes: d391c5827107 ("drivers/firmware: move x86 Generic System Framebuffers support") Reported-by: Ilya Trukhanov Cc: # 5.15.x Signed-off-by: Javier Martinez Canillas --- Changes in v4: - Only fail to probe if a registered fbdev has overlapping aperture (Geert). Changes in v3: - Cc since a Fixes: tag is not enough (gregkh). Changes in v2: - Add a Link: tag with a reference to the bug report (Thorsten Leemhuis). - Add a comment explaining why the probe fails earlier (Daniel Vetter). - Add a Fixes: tag for stable to pick the fix (Daniel Vetter). - Add Daniel Vetter's Reviewed-by: tag. - Improve the commit message and mention the culprit commit drivers/video/fbdev/core/fbmem.c | 16 drivers/video/fbdev/efifb.c | 11 +++ drivers/video/fbdev/simplefb.c | 11 +++ include/linux/fb.h | 1 + 4 files changed, 39 insertions(+) diff --git drivers/video/fbdev/core/fbmem.c drivers/video/fbdev/core/fbmem.c index 826175ad88a2..9906b83132cb 100644 --- drivers/video/fbdev/core/fbmem.c +++ drivers/video/fbdev/core/fbmem.c @@ -1546,6 +1546,22 @@ static bool fb_do_apertures_overlap(struct apertures_struct *gena, return false; } +bool fb_aperture_registered(struct apertures_struct *a) +{ + int i; + + for_each_registered_fb(i) { + struct apertures_struct *gen_aper; + + gen_aper = registered_fb[i]->apertures; + if (fb_do_apertures_overlap(gen_aper, a)) + return true; + } + + return false; +} +EXPORT_SYMBOL(fb_aperture_registered); + static void do_unregister_framebuffer(struct fb_info *fb_info); #define VGA_FB_PHYS 0xA diff --git drivers/video/fbdev/efifb.c drivers/video/fbdev/efifb.c index edca3703b964..1ad6698b2e05 100644 --- drivers/video/fbdev/efifb.c +++ drivers/video/fbdev/efifb.c @@ -457,6 +457,17 @@ static int efifb_probe(struct platform_device *dev) info->apertures->ranges[0].base = efifb_fix.smem_start; info->apertures->ranges[0].size = size_remap; + /* +* Generic drivers must not be registered if a framebuffer exists. +* If a native driver was probed, the display hardware was already +*
[PATCH] drm/bridge: megachips: Ensure both bridges are probed before registration
In the configuration used by the b850v3, the STDP2690 is used to read EDID data whilst it's the STDP4028 which can detect when monitors are connected. This can result in problems at boot with monitors connected when the STDP4028 is probed first, a monitor is detected and an attempt is made to read the EDID data before the STDP2690 has probed: [3.795721] Unable to handle kernel NULL pointer dereference at virtual address 0018 [3.803845] pgd = (ptrval) [3.806581] [0018] *pgd= [3.810180] Internal error: Oops: 5 [#1] SMP ARM [3.814813] Modules linked in: [3.817879] CPU: 0 PID: 64 Comm: kworker/u4:1 Not tainted 5.15.0 #1 [3.824161] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) [3.830705] Workqueue: events_unbound deferred_probe_work_func [3.836565] PC is at stdp2690_get_edid+0x44/0x19c [3.841286] LR is at ge_b850v3_lvds_get_modes+0x2c/0x5c [3.846526] pc : [<805eae10>]lr : [<805eb138>]psr: 8013 [3.852802] sp : 81c359d0 ip : 7dbb550b fp : 81c35a1c [3.858037] r10: 81c73840 r9 : 81c73894 r8 : 816d9800 [3.863270] r7 : r6 : 81c34000 r5 : r4 : 810c35f0 [3.869808] r3 : 80e3e294 r2 : 0080 r1 : 0cc0 r0 : 81401180 [3.876349] Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [3.883499] Control: 10c5387d Table: 1000404a DAC: 0051 [3.889254] Register r0 information: slab kmem_cache start 81401180 pointer offset 0 [3.897034] Register r1 information: non-paged memory [3.902097] Register r2 information: non-paged memory [3.907160] Register r3 information: non-slab/vmalloc memory [3.912832] Register r4 information: non-slab/vmalloc memory [3.918503] Register r5 information: NULL pointer [3.923217] Register r6 information: non-slab/vmalloc memory [3.928887] Register r7 information: NULL pointer [3.933601] Register r8 information: slab kmalloc-1k start 816d9800 pointer offset 0 size 1024 [3.942244] Register r9 information: slab kmalloc-2k start 81c73800 pointer offset 148 size 2048 [3.951058] Register r10 information: slab kmalloc-2k start 81c73800 pointer offset 64 size 2048 [3.959873] Register r11 information: non-slab/vmalloc memory [3.965632] Register r12 information: non-paged memory [3.970781] Process kworker/u4:1 (pid: 64, stack limit = 0x(ptrval)) [3.977148] Stack: (0x81c359d0 to 0x81c36000) [3.981517] 59c0: 80b2b668 80b2b5bc 02e2 034e [3.989712] 59e0: 81c35a8c 816d98e8 81c35a14 7dbb550b 805bfcd0 810c35f0 81c73840 824addc0 [3.997906] 5a00: 1000 816d9800 81c73894 81c73840 81c35a34 81c35a20 805eb138 805eadd8 [4.006099] 5a20: 810c35f0 0045 81c35adc 81c35a38 80594188 805eb118 80d7c788 80dd1848 [4.014292] 5a40: 81c35a50 80dca950 811194d3 80dca7c4 80dca944 80dca91c 816d9800 [4.022485] 5a60: 81c34000 81c760a8 816d9800 80c58c98 810c35f0 816d98e8 1000 1000 [4.030678] 5a80: 8017712c 81c6 0002 0001 [4.038870] 5aa0: 816d9900 816d9900 7dbb550b 805c700c 0008 826282c8 826282c8 [4.047062] 5ac0: 1000 81e1ce40 1000 0002 81c35bf4 81c35ae0 805d9694 80593fc0 [4.055255] 5ae0: 8017a970 80179ad8 0179 81c35bcc 81c35b00 80177108 8017a950 [4.063447] 5b00: 81c35b10 81c34000 81004fd8 81010a38 0059 [4.071639] 5b20: 816d98d4 81fbb718 0013 826282c8 8017a940 81c35b40 81134448 0400 [4.079831] 5b40: 0178 e063b9c1 c249 0040 0008 [4.088024] 5b60: 82628300 82628380 81c34000 81fbb700 82628340 [4.096216] 5b80: 826283c0 1000 0010 816d9800 826282c0 801766f8 [4.104408] 5ba0: 81004fd8 0049 0001 80dcf940 80178de4 [4.112601] 5bc0: 81c35c0c 7dbb550b 80178de4 81fbb700 0010 0010 810c35f4 81e1ce40 [4.120793] 5be0: 81c40908 000c 81c35c64 81c35bf8 805a7f18 805d94a0 81c35c3c 816d9800 [4.128985] 5c00: 0010 81c34000 81c35c2c 81c35c18 8012fce0 805be90c 81c35c3c 81c35c28 [4.137178] 5c20: 805be90c 80173210 81fbb600 81fbb6b4 81c35c5c 7dbb550b 81c35c64 81fbb700 [4.145370] 5c40: 816d9800 0010 810c35f4 81e1ce40 81c40908 000c 81c35c84 81c35c68 [4.153565] 5c60: 805a8c78 805a7ed0 816d9800 81fbb700 0010 81c35cac 81c35c88 [4.161758] 5c80: 805a8dc4 805a8b68 816d9800 816d9800 8179f810 810c42d0 [4.169950] 5ca0: 81c35ccc 81c35cb0 805e47b0 805a8d18 824aa240 81e1ea80 81c40908 81126b60 [4.178144] 5cc0: 81c35d14 81c35cd0 8060db1c 805e46cc 81c35d14 81c35ce0 80dd90f8 810c4d58 [4.186338] 5ce0: 80dd90dc 81fe9740 fffe 81fe9740 81e1ea80 810c4d6c 80c4b95c [4.194531] 5d00: 80dd9a3c 815c6810 81c35d34 81c35d18 8060dc9c 8060d8fc 8246b440 815c6800 [
Re: [PATCH 08/15] iio: buffer-dma: split iio_dma_buffer_fileio_free() function
On Mon, Nov 15, 2021 at 4:20 PM Paul Cercueil wrote: > > From: Alexandru Ardelean > > A part of the logic in the iio_dma_buffer_exit() is required for the change > to add mmap support to IIO buffers. > This change splits the logic into a separate function, which will be > re-used later. > Not sure how the protocol is here, since my old @analog.com email isn't working anymore. But: Signed-off-by: Alexandru Ardelean Thanks :) Alex > Signed-off-by: Alexandru Ardelean > Signed-off-by: Paul Cercueil > --- > drivers/iio/buffer/industrialio-buffer-dma.c | 39 +++- > 1 file changed, 22 insertions(+), 17 deletions(-) > > diff --git a/drivers/iio/buffer/industrialio-buffer-dma.c > b/drivers/iio/buffer/industrialio-buffer-dma.c > index d6b2e0cf..eb8cfd3af030 100644 > --- a/drivers/iio/buffer/industrialio-buffer-dma.c > +++ b/drivers/iio/buffer/industrialio-buffer-dma.c > @@ -358,6 +358,27 @@ int iio_dma_buffer_request_update(struct iio_buffer > *buffer) > } > EXPORT_SYMBOL_GPL(iio_dma_buffer_request_update); > > +static void iio_dma_buffer_fileio_free(struct iio_dma_buffer_queue *queue) > +{ > + unsigned int i; > + > + spin_lock_irq(>list_lock); > + for (i = 0; i < ARRAY_SIZE(queue->fileio.blocks); i++) { > + if (!queue->fileio.blocks[i]) > + continue; > + queue->fileio.blocks[i]->state = IIO_BLOCK_STATE_DEAD; > + } > + spin_unlock_irq(>list_lock); > + > + for (i = 0; i < ARRAY_SIZE(queue->fileio.blocks); i++) { > + if (!queue->fileio.blocks[i]) > + continue; > + iio_buffer_block_put(queue->fileio.blocks[i]); > + queue->fileio.blocks[i] = NULL; > + } > + queue->fileio.active_block = NULL; > +} > + > static void iio_dma_buffer_submit_block(struct iio_dma_buffer_queue *queue, > struct iio_dma_buffer_block *block) > { > @@ -681,25 +702,9 @@ EXPORT_SYMBOL_GPL(iio_dma_buffer_init); > */ > void iio_dma_buffer_exit(struct iio_dma_buffer_queue *queue) > { > - unsigned int i; > - > mutex_lock(>lock); > > - spin_lock_irq(>list_lock); > - for (i = 0; i < ARRAY_SIZE(queue->fileio.blocks); i++) { > - if (!queue->fileio.blocks[i]) > - continue; > - queue->fileio.blocks[i]->state = IIO_BLOCK_STATE_DEAD; > - } > - spin_unlock_irq(>list_lock); > - > - for (i = 0; i < ARRAY_SIZE(queue->fileio.blocks); i++) { > - if (!queue->fileio.blocks[i]) > - continue; > - iio_buffer_block_put(queue->fileio.blocks[i]); > - queue->fileio.blocks[i] = NULL; > - } > - queue->fileio.active_block = NULL; > + iio_dma_buffer_fileio_free(queue); > queue->ops = NULL; > > mutex_unlock(>lock); > -- > 2.33.0 >
Re: [PATCH 14/15] iio: buffer-dmaengine: Add support for cyclic buffers
On Mon, Nov 15, 2021 at 4:23 PM Paul Cercueil wrote: > > Handle the IIO_BUFFER_DMABUF_CYCLIC flag to support cyclic buffers. > Reviewed-by: Alexandru Ardelean > Signed-off-by: Paul Cercueil > --- > drivers/iio/buffer/industrialio-buffer-dma.c | 1 + > .../iio/buffer/industrialio-buffer-dmaengine.c| 15 --- > include/linux/iio/buffer-dma.h| 3 +++ > 3 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/iio/buffer/industrialio-buffer-dma.c > b/drivers/iio/buffer/industrialio-buffer-dma.c > index fb39054d8c15..6658f103ee17 100644 > --- a/drivers/iio/buffer/industrialio-buffer-dma.c > +++ b/drivers/iio/buffer/industrialio-buffer-dma.c > @@ -933,6 +933,7 @@ int iio_dma_buffer_enqueue_dmabuf(struct iio_buffer > *buffer, > } > > dma_block->bytes_used = iio_dmabuf->bytes_used ?: dma_block->size; > + dma_block->cyclic = iio_dmabuf->flags & IIO_BUFFER_DMABUF_CYCLIC; > > switch (dma_block->state) { > case IIO_BLOCK_STATE_QUEUED: > diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c > b/drivers/iio/buffer/industrialio-buffer-dmaengine.c > index 57a8b2e4ba3c..952e2160a11e 100644 > --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c > +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c > @@ -81,9 +81,18 @@ static int iio_dmaengine_buffer_submit_block(struct > iio_dma_buffer_queue *queue, > if (!block->bytes_used || block->bytes_used > max_size) > return -EINVAL; > > - desc = dmaengine_prep_slave_single(dmaengine_buffer->chan, > - block->phys_addr, block->bytes_used, dma_dir, > - DMA_PREP_INTERRUPT); > + if (block->cyclic) { > + desc = dmaengine_prep_dma_cyclic(dmaengine_buffer->chan, > +block->phys_addr, > +block->size, > +block->bytes_used, > +dma_dir, 0); > + } else { > + desc = dmaengine_prep_slave_single(dmaengine_buffer->chan, > + block->phys_addr, > + block->bytes_used, dma_dir, > + DMA_PREP_INTERRUPT); > + } > if (!desc) > return -ENOMEM; > > diff --git a/include/linux/iio/buffer-dma.h b/include/linux/iio/buffer-dma.h > index 85e55fe35282..27639fdf7b54 100644 > --- a/include/linux/iio/buffer-dma.h > +++ b/include/linux/iio/buffer-dma.h > @@ -42,6 +42,7 @@ enum iio_block_state { > * @phys_addr: Physical address of the blocks memory > * @queue: Parent DMA buffer queue > * @state: Current state of the block > + * @cyclic: True if this is a cyclic buffer > * @fileio: True if this buffer is used for fileio mode > * @dmabuf: Underlying DMABUF object > */ > @@ -65,6 +66,8 @@ struct iio_dma_buffer_block { > */ > enum iio_block_state state; > > + bool cyclic; > + > bool fileio; > struct dma_buf *dmabuf; > }; > -- > 2.33.0 >
Re: [PATCH 13/15] iio: core: Add support for cyclic buffers
On Mon, Nov 15, 2021 at 4:22 PM Paul Cercueil wrote: > > Introduce a new flag IIO_BUFFER_DMABUF_CYCLIC in the "flags" field of > the iio_dmabuf uapi structure. > > When set, the DMABUF enqueued with the enqueue ioctl will be endlessly > repeated on the TX output, until the buffer is disabled. > Reviewed-by: Alexandru Ardelean > Signed-off-by: Paul Cercueil > --- > drivers/iio/industrialio-buffer.c | 5 + > include/uapi/linux/iio/buffer.h | 3 ++- > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/iio/industrialio-buffer.c > b/drivers/iio/industrialio-buffer.c > index 30910e6c2346..41bc51c88002 100644 > --- a/drivers/iio/industrialio-buffer.c > +++ b/drivers/iio/industrialio-buffer.c > @@ -1600,6 +1600,11 @@ static int iio_buffer_enqueue_dmabuf(struct iio_buffer > *buffer, > if (dmabuf.flags & ~IIO_BUFFER_DMABUF_SUPPORTED_FLAGS) > return -EINVAL; > > + /* Cyclic flag is only supported on output buffers */ > + if ((dmabuf.flags & IIO_BUFFER_DMABUF_CYCLIC) && > + buffer->direction != IIO_BUFFER_DIRECTION_OUT) > + return -EINVAL; > + > return buffer->access->enqueue_dmabuf(buffer, ); > } > > diff --git a/include/uapi/linux/iio/buffer.h b/include/uapi/linux/iio/buffer.h > index e4621b926262..2d541d038c02 100644 > --- a/include/uapi/linux/iio/buffer.h > +++ b/include/uapi/linux/iio/buffer.h > @@ -7,7 +7,8 @@ > > #include > > -#define IIO_BUFFER_DMABUF_SUPPORTED_FLAGS 0x > +#define IIO_BUFFER_DMABUF_CYCLIC (1 << 0) > +#define IIO_BUFFER_DMABUF_SUPPORTED_FLAGS 0x0001 > > /** > * struct iio_dmabuf_alloc_req - Descriptor for allocating IIO DMABUFs > -- > 2.33.0 >
[syzbot] KASAN: use-after-free Read in drm_gem_object_release_handle
Hello, syzbot found the following issue on: HEAD commit:8ab774587903 Merge tag 'trace-v5.16-5' of git://git.kernel.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=1174ace6b0 kernel config: https://syzkaller.appspot.com/x/.config?x=6d3b8fd1977c1e73 dashboard link: https://syzkaller.appspot.com/bug?extid=c8ae65286134dd1b800d compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2 userspace arch: i386 Unfortunately, I don't have any reproducer for this issue yet. IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+c8ae65286134dd1b8...@syzkaller.appspotmail.com == BUG: KASAN: use-after-free in drm_gem_object_release_handle+0xf2/0x110 drivers/gpu/drm/drm_gem.c:252 Read of size 8 at addr 888028419a28 by task syz-executor.2/10905 CPU: 0 PID: 10905 Comm: syz-executor.2 Not tainted 5.16.0-rc1-syzkaller #0 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014 Call Trace: __dump_stack lib/dump_stack.c:88 [inline] dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106 print_address_description.constprop.0.cold+0x8d/0x320 mm/kasan/report.c:247 __kasan_report mm/kasan/report.c:433 [inline] kasan_report.cold+0x83/0xdf mm/kasan/report.c:450 drm_gem_object_release_handle+0xf2/0x110 drivers/gpu/drm/drm_gem.c:252 idr_for_each+0x113/0x220 lib/idr.c:208 drm_gem_release+0x22/0x30 drivers/gpu/drm/drm_gem.c:930 drm_file_free.part.0+0x805/0xb80 drivers/gpu/drm/drm_file.c:281 drm_file_free drivers/gpu/drm/drm_file.c:248 [inline] drm_close_helper.isra.0+0x17d/0x1f0 drivers/gpu/drm/drm_file.c:308 drm_release+0x1e6/0x530 drivers/gpu/drm/drm_file.c:495 __fput+0x286/0x9f0 fs/file_table.c:280 task_work_run+0xdd/0x1a0 kernel/task_work.c:164 tracehook_notify_resume include/linux/tracehook.h:189 [inline] exit_to_user_mode_loop kernel/entry/common.c:175 [inline] exit_to_user_mode_prepare+0x27e/0x290 kernel/entry/common.c:207 __syscall_exit_to_user_mode_work kernel/entry/common.c:289 [inline] syscall_exit_to_user_mode+0x19/0x60 kernel/entry/common.c:300 __do_fast_syscall_32+0x72/0xf0 arch/x86/entry/common.c:181 do_fast_syscall_32+0x2f/0x70 arch/x86/entry/common.c:203 entry_SYSENTER_compat_after_hwframe+0x4d/0x5c RIP: 0023:0xf6f4e549 Code: 03 74 c0 01 10 05 03 74 b8 01 10 06 03 74 b4 01 10 07 03 74 b0 01 10 08 03 74 d8 01 00 00 00 00 00 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90 90 90 90 8d b4 26 00 00 00 00 8d b4 26 00 00 00 00 RSP: 002b:ff954ef0 EFLAGS: 0282 ORIG_RAX: 0006 RAX: RBX: 0003 RCX: 0002 RDX: RSI: f7084000 RDI: f70aafac RBP: f7084000 R08: R09: R10: R11: R12: R13: R14: R15: Allocated by task 10906: kasan_save_stack+0x1e/0x50 mm/kasan/common.c:38 kasan_set_track mm/kasan/common.c:46 [inline] set_alloc_info mm/kasan/common.c:434 [inline] kasan_kmalloc mm/kasan/common.c:513 [inline] kasan_kmalloc mm/kasan/common.c:472 [inline] __kasan_kmalloc+0xa9/0xd0 mm/kasan/common.c:522 kmalloc include/linux/slab.h:590 [inline] kzalloc include/linux/slab.h:724 [inline] __drm_gem_shmem_create+0x3d8/0x470 drivers/gpu/drm/drm_gem_shmem_helper.c:56 drm_gem_shmem_create drivers/gpu/drm/drm_gem_shmem_helper.c:116 [inline] drm_gem_shmem_create_with_handle+0x26/0x100 drivers/gpu/drm/drm_gem_shmem_helper.c:422 drm_gem_shmem_dumb_create+0x13f/0x290 drivers/gpu/drm/drm_gem_shmem_helper.c:538 drm_mode_create_dumb+0x26c/0x2f0 drivers/gpu/drm/drm_dumb_buffers.c:96 drm_ioctl_kernel+0x27d/0x4e0 drivers/gpu/drm/drm_ioctl.c:782 drm_ioctl+0x51e/0x9d0 drivers/gpu/drm/drm_ioctl.c:885 drm_compat_ioctl+0x270/0x330 drivers/gpu/drm/drm_ioc32.c:987 __do_compat_sys_ioctl+0x1c7/0x290 fs/ioctl.c:972 do_syscall_32_irqs_on arch/x86/entry/common.c:112 [inline] __do_fast_syscall_32+0x65/0xf0 arch/x86/entry/common.c:178 do_fast_syscall_32+0x2f/0x70 arch/x86/entry/common.c:203 entry_SYSENTER_compat_after_hwframe+0x4d/0x5c Freed by task 10906: kasan_save_stack+0x1e/0x50 mm/kasan/common.c:38 kasan_set_track+0x21/0x30 mm/kasan/common.c:46 kasan_set_free_info+0x20/0x30 mm/kasan/generic.c:370 kasan_slab_free mm/kasan/common.c:366 [inline] kasan_slab_free mm/kasan/common.c:328 [inline] __kasan_slab_free+0xff/0x130 mm/kasan/common.c:374 kasan_slab_free include/linux/kasan.h:235 [inline] slab_free_hook mm/slub.c:1723 [inline] slab_free_freelist_hook+0x8b/0x1c0 mm/slub.c:1749 slab_free mm/slub.c:3513 [inline] kfree+0xf6/0x560 mm/slub.c:4561 drm_gem_object_free+0x58/0x80 drivers/gpu/drm/drm_gem.c:972 kref_put include/linux/kref.h:65 [inline] __drm_gem_object_put include/drm/drm_gem.h:371 [inline] drm_gem_object_put include/drm/drm_gem.h:384 [inline]
Re: Backlight control broken on UM325 (OLED) on 5.15 (bisected)
Hi Alex, thank you for your response. On 15.11.2021 10:43, Alex Deucher wrote: > [...] > > That patch adds support for systems with multiple backlights. Do you > have multiple backlight devices now? If so, does the other one work? No, there is still only one backlight device -- amdgpu_bl0. > > Can you also try this patch? > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c > index 4811b0faafd9..67163c9d49e6 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c > @@ -854,8 +854,8 @@ int amdgpu_acpi_init(struct amdgpu_device *adev) > if (amdgpu_device_has_dc_support(adev)) { > #if defined(CONFIG_DRM_AMD_DC) > struct amdgpu_display_manager *dm = >dm; > - if (dm->backlight_dev[0]) > - atif->bd = dm->backlight_dev[0]; > + if (dm->backlight_dev[1]) > + atif->bd = dm->backlight_dev[1]; > #endif > } else { > struct drm_encoder *tmp; > There is no difference in behaviour after applying the patch. Samuel > > Alex > > > > > Regards, > > Samuel Čavoj > > > > [0]: > > https://www.reddit.com/r/AMDLaptops/comments/qst0fm/after_updating_to_linux_515_my_brightness/
[PATCH] gpu/drm: fix potential memleak in error branch
This patch try to fix potential memleak in error branch. Signed-off-by: Bernard Zhao --- drivers/gpu/drm/drm_dp_mst_topology.c | 22 -- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index f3d79eda94bb..f73b180dee73 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -5501,7 +5501,10 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr, int max_lane_count, int max_link_rate, int conn_base_id) { - struct drm_dp_mst_topology_state *mst_state; + struct drm_dp_mst_topology_state *mst_state = NULL; + + mgr->payloads = NULL; + mgr->proposed_vcpis = NULL; mutex_init(>lock); mutex_init(>qlock); @@ -5523,7 +5526,7 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr, */ mgr->delayed_destroy_wq = alloc_ordered_workqueue("drm_dp_mst_wq", 0); if (mgr->delayed_destroy_wq == NULL) - return -ENOMEM; + goto out; INIT_WORK(>work, drm_dp_mst_link_probe_work); INIT_WORK(>tx_work, drm_dp_tx_work); @@ -5539,18 +5542,18 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr, mgr->conn_base_id = conn_base_id; if (max_payloads + 1 > sizeof(mgr->payload_mask) * 8 || max_payloads + 1 > sizeof(mgr->vcpi_mask) * 8) - return -EINVAL; + goto failed; mgr->payloads = kcalloc(max_payloads, sizeof(struct drm_dp_payload), GFP_KERNEL); if (!mgr->payloads) - return -ENOMEM; + goto failed; mgr->proposed_vcpis = kcalloc(max_payloads, sizeof(struct drm_dp_vcpi *), GFP_KERNEL); if (!mgr->proposed_vcpis) - return -ENOMEM; + goto failed; set_bit(0, >payload_mask); mst_state = kzalloc(sizeof(*mst_state), GFP_KERNEL); if (mst_state == NULL) - return -ENOMEM; + goto failed; mst_state->total_avail_slots = 63; mst_state->start_slot = 1; @@ -5563,6 +5566,13 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr, _dp_mst_topology_state_funcs); return 0; + +failed: + kfree(mgr->proposed_vcpis); + kfree(mgr->payloads); + destroy_workqueue(mgr->delayed_destroy_wq); +out: + return -ENOMEM; } EXPORT_SYMBOL(drm_dp_mst_topology_mgr_init); -- 2.33.1
Re: [PATCH v10, 00/19] Support multi hardware decode using of_platform_populate
Hi Ezequiel, On Tue, 2021-11-16 at 09:06 -0300, Ezequiel Garcia wrote: > Hi Yunfei, > > On Tue, 16 Nov 2021 at 08:42, yunfei.d...@mediatek.com > wrote: > > > > Hi Ezequiel, > > > > Thanks for you suggestion. > > On Sun, 2021-11-14 at 19:04 -0300, Ezequiel Garcia wrote: > > > Hi Yunfei, > > > > > > On Thu, 11 Nov 2021 at 01:15, Yunfei Dong < > > > yunfei.d...@mediatek.com> > > > wrote: > > > > > > > > This series adds support for multi hardware decode into mtk- > > > > vcodec, > > > > by first adding use > > > > of_platform_populate to manage each hardware information: > > > > interrupt, clock, register > > > > bases and power. Secondly add core work queue to deal with core > > > > hardware message, > > > > at the same time, add msg queue for different hardware share > > > > messages. Lastly, the > > > > architecture of different specs are not the same, using specs > > > > type > > > > to separate them. > > > > > > > > This series has been tested with both MT8183 and MT8173. > > > > Decoding > > > > was working for both chips. > > > > > > > > > > How are you testing Decoding? If you are running some test suite, > > > it > > > would > > > be good to add such information. > > > > > > > For 8183 and 8173, these patches don't have influence for them, can > > work well. > > > > These patches are used for mt8192 and 8195. We do cts/gts/stress > > test > > to check our drivers. > > What is the meaning of cts/gts/stress tests? > Could you share the sources of the test? > CTS is android compatibility test suite. You can get the information from: https://source.android.com/compatibility. GTS is google Mobile Service Test Suite. Need to run the test suites to test vcodec drivers. Regards them as google android certification. > Thanks, > Ezequiel > Thanks, Yunfei Dong > > > Are you testing some edge-cases such as parallel/concurrent > > > decoding, > > > removing the driver while streaming, and so on? This should help > > > catch > > > some typical issues. > > > > > > Thanks, > > > Ezequiel > > > > > > > Thanks, > > Yunfei Dong > > > > Patches 1~3 rewrite get register bases and power on/off > > > > interface. > > > > Patches 4 export decoder pm interfaces. > > > > Patches 5 add to support 8192. > > > > Patch 6 support multi hardware. > > > > Patch 7 separate video encoder and decoder document > > > > Patch 8-17 add interfaces to support core hardware. > > > > Patch 18-19 remove mtk_vcodec_release_dec/enc_pm interfaces. > > > > --- > > > > changes compared with v9: > > > > - need not to build ko, just export pm interfaces for patch > > > > 04/19. > > > > - fix comments for patch 06/19 > > > > > > > > changes compared with v8: > > > > - add new patch 18~19 to remove mtk_vcodec_release_de/enc_pm > > > > interfaces. > > > > - fix spelling mistakes for patch 17/19 > > > > - fix yaml comments for patch 15/19 > > > > > > > > Changes compared with v7: > > > > - add new patch 4 to build decoder pm file as module > > > > - add new patch 5 to support 8192 > > > > - fix comments for patch 6/17 > > > > - change some logic for using work queue instead of create > > > > thread > > > > for core hardware decode for patch 10/17 > > > > - using work queue for hardware decode instead of create thread > > > > for > > > > patch 13/17 > > > > - add returen value for patch 14/17 > > > > - fix yaml check fail 15/17 > > > > > > > > Changes compared with v6: > > > > - Use of_platform_populate to manage multi hardware, not > > > > component > > > > framework for patch 4/15 > > > > - Re-write dtsi document for hardware architecture changed for > > > > patch 13/15 -The dtsi will write like below in patch 13/15: > > > > vcodec_dec: vcodec_dec@1600 { > > > > compatible = "mediatek,mt8192-vcodec-dec"; > > > > #address-cells = <2>; > > > > #size-cells = <2>; > > > > ranges; > > > > reg = <0 0x1600 0 0x1000>; /* VDEC_SYS */ > > > > mediatek,scp = <>; > > > > iommus = < M4U_PORT_L4_VDEC_MC_EXT>; > > > > dma-ranges = <0x1 0x0 0x0 0x4000 0x0 0xfff0>; > > > > vcodec_lat { > > > > compatible = "mediatek,mtk-vcodec-lat"; > > > > reg = <0 0x1601 0 0x800>; /* > > > > VDEC_MISC */ > > > > reg-name = "reg-misc"; > > > > interrupts = ; > > > > iommus = < M4U_PORT_L5_VDEC_LAT0_VLD_EXT>, > > > > < M4U_PORT_L5_VDEC_LAT0_VLD2_EXT>, > > > > < M4U_PORT_L5_VDEC_LAT0_AVC_MV_EXT>, > > > > < M4U_PORT_L5_VDEC_LAT0_PRED_RD_EXT>, > > > > < M4U_PORT_L5_VDEC_LAT0_TILE_EXT>, > > > > < M4U_PORT_L5_VDEC_LAT0_WDMA_EXT>, > > > > < > > > > M4U_PORT_L5_VDEC_LAT0_RG_CTRL_DMA_EXT>, > > > > < M4U_PORT_L5_VDEC_UFO_ENC_EXT>; > > > > clocks = < CLK_TOP_VDEC_SEL>, > > > > <_soc CLK_VDEC_SOC_VDEC>, > > > > <_soc CLK_VDEC_SOC_LAT>,
[bug report] drm/msm/dpu: merge struct dpu_irq into struct dpu_hw_intr
Hello Dmitry Baryshkov, The patch f25f656608e3: "drm/msm/dpu: merge struct dpu_irq into struct dpu_hw_intr" from Jun 18, 2021, leads to the following Smatch static checker warnings: drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c:569 dpu_core_irq_preinstall() error: potential null dereference 'dpu_kms->hw_intr->irq_cb_tbl'. (kcalloc returns null) drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c:570 dpu_core_irq_preinstall() error: potential null dereference 'dpu_kms->hw_intr->irq_counts'. (kcalloc returns null) drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c 554 void dpu_core_irq_preinstall(struct dpu_kms *dpu_kms) 555 { 556 int i; 557 558 pm_runtime_get_sync(_kms->pdev->dev); 559 dpu_clear_irqs(dpu_kms); 560 dpu_disable_all_irqs(dpu_kms); 561 pm_runtime_put_sync(_kms->pdev->dev); 562 563 /* Create irq callbacks for all possible irq_idx */ 564 dpu_kms->hw_intr->irq_cb_tbl = kcalloc(dpu_kms->hw_intr->total_irqs, 565 sizeof(struct list_head), GFP_KERNEL); 566 dpu_kms->hw_intr->irq_counts = kcalloc(dpu_kms->hw_intr->total_irqs, 567 sizeof(atomic_t), GFP_KERNEL); No checks for these kcallocs... It's not actually new code but shuffling them around makes them show up as new in my tests. 568 for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++) { --> 569 INIT_LIST_HEAD(_kms->hw_intr->irq_cb_tbl[i]); 570 atomic_set(_kms->hw_intr->irq_counts[i], 0); 571 } 572 } regards, dan carpenter
Re: [PATCH v10, 00/19] Support multi hardware decode using of_platform_populate
Hi Yunfei, On Tue, 16 Nov 2021 at 08:42, yunfei.d...@mediatek.com wrote: > > Hi Ezequiel, > > Thanks for you suggestion. > On Sun, 2021-11-14 at 19:04 -0300, Ezequiel Garcia wrote: > > Hi Yunfei, > > > > On Thu, 11 Nov 2021 at 01:15, Yunfei Dong > > wrote: > > > > > > This series adds support for multi hardware decode into mtk-vcodec, > > > by first adding use > > > of_platform_populate to manage each hardware information: > > > interrupt, clock, register > > > bases and power. Secondly add core work queue to deal with core > > > hardware message, > > > at the same time, add msg queue for different hardware share > > > messages. Lastly, the > > > architecture of different specs are not the same, using specs type > > > to separate them. > > > > > > This series has been tested with both MT8183 and MT8173. Decoding > > > was working for both chips. > > > > > > > How are you testing Decoding? If you are running some test suite, it > > would > > be good to add such information. > > > For 8183 and 8173, these patches don't have influence for them, can > work well. > > These patches are used for mt8192 and 8195. We do cts/gts/stress test > to check our drivers. What is the meaning of cts/gts/stress tests? Could you share the sources of the test? Thanks, Ezequiel > > Are you testing some edge-cases such as parallel/concurrent decoding, > > removing the driver while streaming, and so on? This should help > > catch > > some typical issues. > > > > Thanks, > > Ezequiel > > > Thanks, > Yunfei Dong > > > Patches 1~3 rewrite get register bases and power on/off interface. > > > Patches 4 export decoder pm interfaces. > > > Patches 5 add to support 8192. > > > Patch 6 support multi hardware. > > > Patch 7 separate video encoder and decoder document > > > Patch 8-17 add interfaces to support core hardware. > > > Patch 18-19 remove mtk_vcodec_release_dec/enc_pm interfaces. > > > --- > > > changes compared with v9: > > > - need not to build ko, just export pm interfaces for patch 04/19. > > > - fix comments for patch 06/19 > > > > > > changes compared with v8: > > > - add new patch 18~19 to remove mtk_vcodec_release_de/enc_pm > > > interfaces. > > > - fix spelling mistakes for patch 17/19 > > > - fix yaml comments for patch 15/19 > > > > > > Changes compared with v7: > > > - add new patch 4 to build decoder pm file as module > > > - add new patch 5 to support 8192 > > > - fix comments for patch 6/17 > > > - change some logic for using work queue instead of create thread > > > for core hardware decode for patch 10/17 > > > - using work queue for hardware decode instead of create thread for > > > patch 13/17 > > > - add returen value for patch 14/17 > > > - fix yaml check fail 15/17 > > > > > > Changes compared with v6: > > > - Use of_platform_populate to manage multi hardware, not component > > > framework for patch 4/15 > > > - Re-write dtsi document for hardware architecture changed for > > > patch 13/15 -The dtsi will write like below in patch 13/15: > > > vcodec_dec: vcodec_dec@1600 { > > > compatible = "mediatek,mt8192-vcodec-dec"; > > > #address-cells = <2>; > > > #size-cells = <2>; > > > ranges; > > > reg = <0 0x1600 0 0x1000>; /* VDEC_SYS */ > > > mediatek,scp = <>; > > > iommus = < M4U_PORT_L4_VDEC_MC_EXT>; > > > dma-ranges = <0x1 0x0 0x0 0x4000 0x0 0xfff0>; > > > vcodec_lat { > > > compatible = "mediatek,mtk-vcodec-lat"; > > > reg = <0 0x1601 0 0x800>; /* > > > VDEC_MISC */ > > > reg-name = "reg-misc"; > > > interrupts = ; > > > iommus = < M4U_PORT_L5_VDEC_LAT0_VLD_EXT>, > > > < M4U_PORT_L5_VDEC_LAT0_VLD2_EXT>, > > > < M4U_PORT_L5_VDEC_LAT0_AVC_MV_EXT>, > > > < M4U_PORT_L5_VDEC_LAT0_PRED_RD_EXT>, > > > < M4U_PORT_L5_VDEC_LAT0_TILE_EXT>, > > > < M4U_PORT_L5_VDEC_LAT0_WDMA_EXT>, > > > < M4U_PORT_L5_VDEC_LAT0_RG_CTRL_DMA_EXT>, > > > < M4U_PORT_L5_VDEC_UFO_ENC_EXT>; > > > clocks = < CLK_TOP_VDEC_SEL>, > > > <_soc CLK_VDEC_SOC_VDEC>, > > > <_soc CLK_VDEC_SOC_LAT>, > > > <_soc CLK_VDEC_SOC_LARB1>, > > > < CLK_TOP_MAINPLL_D4>; > > > clock-names = "vdec-sel", "vdec-soc-vdec", "vdec-soc- > > > lat", > > > "vdec-vdec", "vdec-top"; > > > assigned-clocks = < CLK_TOP_VDEC_SEL>; > > > assigned-clock-parents = < > > > CLK_TOP_MAINPLL_D4>; > > > power-domains = < MT8192_POWER_DOMAIN_VDEC>; > > > }; > > > > > > vcodec_core { > > > compatible = "mediatek,mtk-vcodec-core"; > > > reg = <0 0x16025000 0 0x1000>; /* > > > VDEC_CORE_MISC */ > > > reg-names = "reg-misc"; > > > interrupts = ; > > > iommus = <
Re: [PATCH v10, 15/19] dt-bindings: media: mtk-vcodec: Adds decoder dt-bindings for mt8192
Hi Ezequiel, Thanks for your suggestion. On Sun, 2021-11-14 at 18:56 -0300, Ezequiel Garcia wrote: > Yunfei, > > On Thu, 11 Nov 2021 at 01:15, Yunfei Dong > wrote: > > > > Adds decoder dt-bindings for mt8192. > > > > Signed-off-by: Yunfei Dong > > --- > > .../media/mediatek,vcodec-subdev-decoder.yaml | 261 > > ++ > > 1 file changed, 261 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/media/mediatek,vcodec-subdev- > > decoder.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev- > > decoder.yaml > > b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev- > > decoder.yaml > > new file mode 100644 > > index ..1886fae6e39d > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec- > > subdev-decoder.yaml > > @@ -0,0 +1,261 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > + > > +%YAML 1.2 > > +--- > > +$id: " > > http://devicetree.org/schemas/media/mediatek,vcodec-subdev-decoder.yaml# > > " > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#; > > + > > +title: Mediatek Video Decode Accelerator With Multi Hardware > > + > > +maintainers: > > + - Yunfei Dong > > + > > +description: | > > + Mediatek Video Decode is the video decode hardware present in > > Mediatek > > + SoCs which supports high resolution decoding functionalities. > > Required > > + main and subdev device node. > > + > > + About the Decoder Hardware Block Diagram, please check below: > > + > > ++-+--- > > -+ > > +| | > > | > > +| input -> lat HW -> lat buffer --|--> lat buffer -> core HW > > -> output > > | > > To be completely honest, I can't really understand what is the > meaning > of the blocks > with input -> lat hw -> lat buffer, and how this means lat- and core- > are children of some parent. > I will add more detail information as below: The block diagram is data flow, need two or more hardwares to decode for each picture. input is input bitstream, lat hardware need input bitstream and lat buffer to decode, will write temp information to lat buffer when lat decode done. lat buffer is used to share lat buffer information for lat hardware and core hardware. core hardware need frame buffer and lat buffer to decode, will write decode result to frame buffer. > > +||| | || > > | > > ++||---+-||-- > > ---+ > > + || lat thread | core > > thread|| > > +-||-||-- > > -- > > + || || > > > > + \/\/ > > + + > > --+ > > + |enable/disable > > | > > + | clk powerirqiommu > > port | > > + | (lat/lat > > soc/core0/core1)| > > + + > > --+ > > + > > + As above, mean in main device, mean in subdev > > device. The information > > + of each hardware will be stored in subdev device. There are two > > workqueues in main device: > > + lat and core. Enable/disable the lat clk/power/irq when lat need > > to work through hardware > > + index, core is the same. > > + > > + Normally the smi common may not the same for each hardware, > > can't combine all hardware in > > + one node, or leading to iommu fault when access dram data. > > + > There are one parent node, regards it as main device, each child node as subdevices, one child node mean one hardware. There are two work queues in main device used for lat and core hardware to decode. The information of each hardware will be stored in subdevices. Enable/disable the lat clk/power/irq when hardware need to work. > To what extent the lat- and core- devices are really "children" > or "subdevices" of the video-codec@1600 device? > > I.e. what resources do they share? What are the details of > their bus topology? > The hardware is really very simple, just enable it to work when needed. But each hardware isn't independent, need lat and core to decode for every picture. No complex bus topology. > > + > > +examples: > > + - | > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +video-codec@1600 { > > +compatible = "mediatek,mt8192-vcodec-dec"; > > +reg = <0x1600 0x1000>; /* VDEC_SYS */ > > +mediatek,scp = <>; > > +iommus = < M4U_PORT_L4_VDEC_MC_EXT>; > > +dma-ranges = <0x1 0x0 0x0
Re: [PATCH v3 0/9] backlight: qcom-wled: fix and solidify handling of enabled-strings
Hi Lee On Mon, Nov 15, 2021 at 09:34:50PM +0100, Marijn Suijten wrote: > This patchset fixes WLED's handling of enabled-strings: besides some > cleanup it is now actually possible to specify a non-contiguous array of > enabled strings (not necessarily starting at zero) and the values from > DT are now validated to prevent possible unexpected out-of-bounds > register and array element accesses. > Off-by-one mistakes in the maximum number of strings, also causing > out-of-bounds access, have been addressed as well. They have arrived piecemeal (during v1, v2 and v3) but all patches on the set should now have my R-b: attached to them. Daniel.
Re: [PATCH v3 5/9] backlight: qcom-wled: Override default length with qcom,enabled-strings
On Mon, Nov 15, 2021 at 09:34:55PM +0100, Marijn Suijten wrote: > The length of qcom,enabled-strings as property array is enough to > determine the number of strings to be enabled, without needing to set > qcom,num-strings to override the default number of strings when less > than the default (which is also the maximum) is provided in DT. > > This also introduces an extra warning when qcom,num-strings is set, > denoting that it is not necessary to set both anymore. It is usually > more concise to set just qcom,num-length when a zero-based, contiguous > range of strings is needed (the majority of the cases), or to only set > qcom,enabled-strings when a specific set of indices is desired. > > Fixes: 775d2ffb4af6 ("backlight: qcom-wled: Restructure the driver for WLED3") > Signed-off-by: Marijn Suijten > Reviewed-by: AngeloGioacchino Del Regno > Reviewed-by: Daniel Thompson Daniel.
Re: [PATCH v3 4/9] backlight: qcom-wled: Fix off-by-one maximum with default num_strings
On Mon, Nov 15, 2021 at 09:34:54PM +0100, Marijn Suijten wrote: > When not specifying num-strings in the DT the default is used, but +1 is > added to it which turns WLED3 into 4 and WLED4/5 into 5 strings instead > of 3 and 4 respectively, causing out-of-bounds reads and register > read/writes. This +1 exists for a deficiency in the DT parsing code, > and is simply omitted entirely - solving this oob issue - by parsing the > property separately much like qcom,enabled-strings. > > This also enables more stringent checks on the maximum value when > qcom,enabled-strings is provided in the DT, by parsing num-strings after > enabled-strings to allow it to check against (and in a subsequent patch > override) the length of enabled-strings: it is invalid to set > num-strings higher than that. > The DT currently utilizes it to get around an incorrect fixed read of > four elements from that array (has been addressed in a prior patch) by > setting a lower num-strings where desired. > > Fixes: 93c64f1ea1e8 ("leds: add Qualcomm PM8941 WLED driver") > Signed-off-by: Marijn Suijten > Reviewed-By: AngeloGioacchino Del Regno > Reviewed-by: Daniel Thompson Daniel.