[PATCH v2] media: mtk-vcodec: Align width and height to 64 bytes
Width and height need to 64 bytes aligned when setting the format.
Need to make sure all is 64 bytes align when use width and height to
calculate buffer size.
Signed-off-by: Yunfei Dong
Change-Id: I39886b1a6b433c92565ddbf297eb193456eec1d2
---
drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h| 1 +
drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h
b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h
index e30806c1faea..66cd6d2242c3 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h
@@ -11,6 +11,7 @@
#include
#include
+#define VCODEC_DEC_ALIGNED_64 64
#define VCODEC_CAPABILITY_4K_DISABLED 0x10
#define VCODEC_DEC_4K_CODED_WIDTH 4096U
#define VCODEC_DEC_4K_CODED_HEIGHT 2304U
diff --git a/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c
b/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c
index d402fc4bda69..e1a3011772a9 100644
--- a/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c
+++ b/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c
@@ -562,8 +562,8 @@ static void get_pic_info(struct vdec_h264_slice_inst *inst,
{
struct mtk_vcodec_ctx *ctx = inst->ctx;
- ctx->picinfo.buf_w = (ctx->picinfo.pic_w + 15) & 0xFFF0;
- ctx->picinfo.buf_h = (ctx->picinfo.pic_h + 31) & 0xFFE0;
+ ctx->picinfo.buf_w = ALIGN(ctx->picinfo.pic_w, VCODEC_DEC_ALIGNED_64);
+ ctx->picinfo.buf_h = ALIGN(ctx->picinfo.pic_h, VCODEC_DEC_ALIGNED_64);
ctx->picinfo.fb_sz[0] = ctx->picinfo.buf_w * ctx->picinfo.buf_h;
ctx->picinfo.fb_sz[1] = ctx->picinfo.fb_sz[0] >> 1;
inst->vsi_ctx.dec.cap_num_planes =
--
2.25.1
Re: [PATCH v1] media: mtk-vcodec: Align width and height to 64
Hi steve,
Thanks for your suggestion.On Tue, 2021-11-02 at 09:43 -0700, Steve Cho
wrote:
> Thank you Yunfei for following up with this change.
> This change is the last missing piece to enable VD on Kukui with
> Chromium.
> This patch fixed the corruption we were seeing on Kukui with certain
> tests.
>
> One comment from me is just to use defined macro or variable instead
> of hard coding 64.
>
> "User get width and height are 64 align when set format."
>
> This sentence might need to be reworded. It is not clear to me.
>
> Maybe something like "Width and height need to be 64 bytes aligned
> when setting the format."
>
> Thanks,Steve
Fix it and send patch v2.
Thanks,Yunfei Dong
> On Fri, Oct 29, 2021 at 2:45 AM Yunfei Dong > wrote:
> > User get width and height are 64 align when set format. Need to
> > make
> >
> > sure all is 64 align when use width and height to calculate buffer
> > size.
> >
> >
> >
> > Signed-off-by: Yunfei Dong
> >
> > ---
> >
> > drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c | 4 ++--
> >
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> >
> >
> > diff --git a/drivers/media/platform/mtk-
> > vcodec/vdec/vdec_h264_req_if.c b/drivers/media/platform/mtk-
> > vcodec/vdec/vdec_h264_req_if.c
> >
> > index 946c23088308..28c17204f9a1 100644
> >
> > --- a/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c
> >
> > +++ b/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c
> >
> > @@ -562,8 +562,8 @@ static void get_pic_info(struct
> > vdec_h264_slice_inst *inst,
> >
> > {
> >
> > struct mtk_vcodec_ctx *ctx = inst->ctx;
> >
> >
> >
> > - ctx->picinfo.buf_w = (ctx->picinfo.pic_w + 15) &
> > 0xFFF0;
> >
> > - ctx->picinfo.buf_h = (ctx->picinfo.pic_h + 31) &
> > 0xFFE0;
> >
> > + ctx->picinfo.buf_w = ALIGN(ctx->picinfo.pic_w, 64);
> >
> > + ctx->picinfo.buf_h = ALIGN(ctx->picinfo.pic_h, 64);
> >
> > ctx->picinfo.fb_sz[0] = ctx->picinfo.buf_w * ctx-
> > >picinfo.buf_h;
> >
> > ctx->picinfo.fb_sz[1] = ctx->picinfo.fb_sz[0] >> 1;
> >
> > inst->vsi_ctx.dec.cap_num_planes =
> >
[Bug 214921] New: amdgpu hangs HP Laptop on shutdown
https://bugzilla.kernel.org/show_bug.cgi?id=214921 Bug ID: 214921 Summary: amdgpu hangs HP Laptop on shutdown Product: Drivers Version: 2.5 Kernel Version: 5.15 Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: spassw...@web.de Regression: No Commit bf756fb833cbe8c6881c964f09db718bade6e591 leads to an improper shutdown, i.e. the System does not switch off and has to be powered off by pressing the power button for a long time. The problem seems to occur relatively late in the shutdown as it leaves no trace in logfiles. It also does not fix hangs on suspension on this Laptop. Reverting this commit in 5.15 leads to working shutdown again while resuming from suspension still does not work. Hardware: HP bw064-ng CPU: processor : 0 vendor_id : AuthenticAMD cpu family : 21 model : 101 model name : AMD A10-9620P RADEON R5, 10 COMPUTE CORES 4C+6G stepping: 1 microcode : 0x6006118 GPU: 00:01.0 VGA compatible controller [0300]: Advanced Micro Devices, Inc. [AMD/ATI] Wani [Radeon R5/R6/R7 Graphics] [1002:9874] (rev ca) -- 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 -V2] drm/sun4i: Grab reference of connector before return connector from sun4i_tcon_get_connector
在 2021/11/2 23:03, Maxime Ripard 写道:
Hi,
On Tue, Nov 02, 2021 at 04:46:28AM -0400, He Ying wrote:
From the comments of drm_for_each_connector_iter(), we know
that "connector is only valid within the list body, if you
want to use connector after calling drm_connector_list_iter_end()
then you need to grab your own reference first using
drm_connector_get()". So fix the wrong use of connector
according to the comments and then call drm_connector_put()
after using connector finishes.
Signed-off-by: He Ying
---
V2:
Use proper subject prefix
drivers/gpu/drm/sun4i/sun4i_tcon.c | 18 +-
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 9f06dec0fc61..24fa6784ee5f 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -47,12 +47,12 @@ static struct drm_connector *sun4i_tcon_get_connector(const
struct drm_encoder *
drm_connector_list_iter_begin(encoder->dev, );
drm_for_each_connector_iter(connector, )
if (connector->encoder == encoder) {
- drm_connector_list_iter_end();
- return connector;
+ drm_connector_get(connector);
+ break;
}
drm_connector_list_iter_end();
- return NULL;
+ return connector;
Connector might be uninitialized if we don't find one here
Connector should be NULL if we don't find one. The code is
#define drm_for_each_connector_iter(connector, iter) \
while ((connector = drm_connector_list_iter_next(iter)))
So, when we don't break from the while body, connector
can only be NULL.
}
static int sun4i_tcon_get_pixel_depth(const struct drm_encoder *encoder)
@@ -65,6 +65,7 @@ static int sun4i_tcon_get_pixel_depth(const struct
drm_encoder *encoder)
return -EINVAL;
info = >display_info;
+ drm_connector_put(connector);
if (info->num_bus_formats != 1)
We're still accessing connector->display_info here, but it might have been
freed already.
Agree. I'll place it after using 'info' finishes in v3.
Maxime
[Bug 214859] drm-amdgpu-init-iommu~fd-device-init.patch introduce bug
https://bugzilla.kernel.org/show_bug.cgi?id=214859 James Zhu (jam...@amd.com) changed: What|Removed |Added CC||jam...@amd.com --- Comment #5 from James Zhu (jam...@amd.com) --- Created attachment 299413 --> https://bugzilla.kernel.org/attachment.cgi?id=299413=edit patch to fix Suggest to upgrade to 5.15rc7 and apply this patch, then make a test. -- 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: [git pull] drm for 5.16-rc1
The pull request you sent on Wed, 3 Nov 2021 09:34:23 +1000: > git://anongit.freedesktop.org/drm/drm tags/drm-next-2021-11-03 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/56d33754481fe0dc7436dc4ee4fbd44b3039361d Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [Intel-gfx] [PATCH v2 2/2] drm/i915/uapi: Add query for hwconfig table
On 11/1/2021 08:39, Jordan Justen wrote:
writes:
From: Rodrigo Vivi
GuC contains a consolidated table with a bunch of information about the
current device.
Previously, this information was spread and hardcoded to all the components
including GuC, i915 and various UMDs. The goal here is to consolidate
the data into GuC in a way that all interested components can grab the
very latest and synchronized information using a simple query.
As per most of the other queries, this one can be called twice.
Once with item.length=0 to determine the exact buffer size, then
allocate the user memory and call it again for to retrieve the
table data. For example:
struct drm_i915_query_item item = {
.query_id = DRM_I915_QUERY_HWCONCFIG_TABLE;
};
query.items_ptr = (int64_t)
query.num_items = 1;
ioctl(fd, DRM_IOCTL_I915_QUERY, query, sizeof(query));
if (item.length <= 0)
return -ENOENT;
data = malloc(item.length);
item.data_ptr = (int64_t)
ioctl(fd, DRM_IOCTL_I915_QUERY, query, sizeof(query));
// Parse the data as appropriate...
The returned array is a simple and flexible KLV (Key/Length/Value)
formatted table. For example, it could be just:
enum device_attr {
ATTR_SOME_VALUE = 0,
ATTR_SOME_MASK = 1,
};
static const u32 hwconfig[] = {
ATTR_SOME_VALUE,
1, // Value Length in DWords
8, // Value
ATTR_SOME_MASK,
3,
0x00, 0x, 0xFF00,
};
Seems simple enough, so why doesn't i915 define the format of the
returned hwconfig blob in i915_drm.h?
Because the definition is nothing to do with i915. This table comes from
the hardware spec. It is not defined by the KMD and it is not currently
used by the KMD. So there is no reason for the KMD to be creating
structures for it in the same way that the KMD does not document,
define, struct, etc. every other feature of the hardware that the UMDs
might use.
struct drm_i915_hwconfig {
uint32_t key;
uint32_t length;
uint32_t values[];
};
It sounds like the kernel depends on the closed source guc being loaded
to return this information. Is that right? Will i915 also become
dependent on some of this data such that it won't be able to initialize
without the firmware being loaded?
At the moment, the KMD does not use the table at all. We merely provide
a mechanism for the UMDs to retrieve it from the hardware.
In terms of future direction, that is something you need to take up with
the hardware architects.
The attribute ids are defined in a hardware spec.
Which spec?
Unfortunately, it is not one that is currently public. We are pushing
the relevant people to get it included in the public bspec / HRM. It is
a slow process :(.
John.
-Jordan
Re: [PATCH v7 15/20] drm/mediatek: add display merge mute/unmute support for MT8195
Hi, Nancy:
Nancy.Lin 於 2021年10月29日 週五 下午3:52寫道：
>
> Add merge mute/unmute setting for MT8195.
> MT8195 Vdosys1 merge1~merge4 support HW mute function.
Reviewed-by: Chun-Kuang Hu
>
> Signed-off-by: Nancy.Lin
> ---
> drivers/gpu/drm/mediatek/mtk_disp_merge.c | 23 ++-
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_merge.c
> b/drivers/gpu/drm/mediatek/mtk_disp_merge.c
> index 92c81ce24a49..dff2797a2f68 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_merge.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_merge.c
> @@ -58,12 +58,15 @@
> #define FLD_PREULTRA_TH_LOWGENMASK(15, 0)
> #define FLD_PREULTRA_TH_HIGH GENMASK(31, 16)
>
> +#define DISP_REG_MERGE_MUTE_0 0xf00
> +
> struct mtk_disp_merge {
> - void __iomem *regs;
> - struct clk *clk;
> - struct clk *async_clk;
> - struct cmdq_client_reg cmdq_reg;
> - boolfifo_en;
> + void __iomem*regs;
> + struct clk *clk;
> + struct clk *async_clk;
> + struct cmdq_client_reg cmdq_reg;
> + boolfifo_en;
> + boolmute_support;
> };
>
> void mtk_merge_start(struct device *dev)
> @@ -82,6 +85,10 @@ void mtk_merge_start_cmdq(struct device *dev, struct
> cmdq_pkt *cmdq_pkt)
> {
> struct mtk_disp_merge *priv = dev_get_drvdata(dev);
>
> + if (priv->mute_support)
> + mtk_ddp_write(cmdq_pkt, 0x0, >cmdq_reg, priv->regs,
> + DISP_REG_MERGE_MUTE_0);
> +
> mtk_ddp_write(cmdq_pkt, 1, >cmdq_reg, priv->regs,
> DISP_REG_MERGE_CTRL);
> }
> @@ -90,6 +97,10 @@ void mtk_merge_stop_cmdq(struct device *dev, struct
> cmdq_pkt *cmdq_pkt)
> {
> struct mtk_disp_merge *priv = dev_get_drvdata(dev);
>
> + if (priv->mute_support)
> + mtk_ddp_write(cmdq_pkt, 0x1, >cmdq_reg, priv->regs,
> + DISP_REG_MERGE_MUTE_0);
> +
> mtk_ddp_write(cmdq_pkt, 0, >cmdq_reg, priv->regs,
> DISP_REG_MERGE_CTRL);
> }
> @@ -264,6 +275,8 @@ static int mtk_disp_merge_probe(struct platform_device
> *pdev)
> priv->fifo_en = of_property_read_bool(dev->of_node,
> "mediatek,merge-fifo-en");
>
> + priv->mute_support = of_property_read_bool(dev->of_node,
> + "mediatek,merge-mute");
> platform_set_drvdata(pdev, priv);
>
> ret = component_add(dev, _disp_merge_component_ops);
> --
> 2.18.0
>
Re: [PATCH v7 14/20] drm/mediatek: add display merge start/stop API for cmdq support
Hi, Nancy:
Nancy.Lin 於 2021年10月29日 週五 下午3:52寫道：
>
> Add merge start/stop API for cmdq support. The ovl_adaptor merges
> are configured with each drm plane update. Need to enable/disable
> merge with cmdq making sure all the settings taken effect in the
> same vblank.
Reviewed-by: Chun-Kuang Hu
>
> Signed-off-by: Nancy.Lin
> ---
> drivers/gpu/drm/mediatek/mtk_disp_drv.h | 2 ++
> drivers/gpu/drm/mediatek/mtk_disp_merge.c | 20 +---
> 2 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> index c2de53a5892e..224a710bb537 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -66,6 +66,8 @@ void mtk_merge_stop(struct device *dev);
> void mtk_merge_advance_config(struct device *dev, unsigned int l_w, unsigned
> int r_w,
> unsigned int h, unsigned int vrefresh, unsigned
> int bpc,
> struct cmdq_pkt *cmdq_pkt);
> +void mtk_merge_start_cmdq(struct device *dev, struct cmdq_pkt *cmdq_pkt);
> +void mtk_merge_stop_cmdq(struct device *dev, struct cmdq_pkt *cmdq_pkt);
>
> void mtk_ovl_bgclr_in_on(struct device *dev);
> void mtk_ovl_bgclr_in_off(struct device *dev);
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_merge.c
> b/drivers/gpu/drm/mediatek/mtk_disp_merge.c
> index 558e0cb2a297..92c81ce24a49 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_merge.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_merge.c
> @@ -67,17 +67,31 @@ struct mtk_disp_merge {
> };
>
> void mtk_merge_start(struct device *dev)
> +{
> + mtk_merge_start_cmdq(dev, NULL);
> +}
> +
> +void mtk_merge_stop(struct device *dev)
> {
> struct mtk_disp_merge *priv = dev_get_drvdata(dev);
>
> - writel(MERGE_EN, priv->regs + DISP_REG_MERGE_CTRL);
> + mtk_merge_stop_cmdq(dev, NULL);
> }
>
> -void mtk_merge_stop(struct device *dev)
> +void mtk_merge_start_cmdq(struct device *dev, struct cmdq_pkt *cmdq_pkt)
> +{
> + struct mtk_disp_merge *priv = dev_get_drvdata(dev);
> +
> + mtk_ddp_write(cmdq_pkt, 1, >cmdq_reg, priv->regs,
> + DISP_REG_MERGE_CTRL);
> +}
> +
> +void mtk_merge_stop_cmdq(struct device *dev, struct cmdq_pkt *cmdq_pkt)
> {
> struct mtk_disp_merge *priv = dev_get_drvdata(dev);
>
> - writel(0x0, priv->regs + DISP_REG_MERGE_CTRL);
> + mtk_ddp_write(cmdq_pkt, 0, >cmdq_reg, priv->regs,
> + DISP_REG_MERGE_CTRL);
> }
>
> static void mtk_merge_fifo_setting(struct mtk_disp_merge *priv,
> --
> 2.18.0
>
Re: [PATCH v7 13/20] drm/mediatek: add display merge advance config API for MT8195
Hi, Nancy:
Nancy.Lin 於 2021年10月29日 週五 下午3:52寫道：
>
> Add merge new advance config API. The original merge API is
> mtk_ddp_comp_funcs function prototype. The API interface parameters
> cannot be modified, so add a new config API for extension. This is
> the preparation for ovl_adaptor merge control.
Reviewed-by: Chun-Kuang Hu
>
> Signed-off-by: Nancy.Lin
> ---
> drivers/gpu/drm/mediatek/mtk_disp_drv.h | 3 ++
> drivers/gpu/drm/mediatek/mtk_disp_merge.c | 52 ---
> 2 files changed, 48 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> index b3a372cab0bd..c2de53a5892e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -63,6 +63,9 @@ void mtk_merge_config(struct device *dev, unsigned int
> width,
> unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
> void mtk_merge_start(struct device *dev);
> void mtk_merge_stop(struct device *dev);
> +void mtk_merge_advance_config(struct device *dev, unsigned int l_w, unsigned
> int r_w,
> + unsigned int h, unsigned int vrefresh, unsigned
> int bpc,
> + struct cmdq_pkt *cmdq_pkt);
>
> void mtk_ovl_bgclr_in_on(struct device *dev);
> void mtk_ovl_bgclr_in_off(struct device *dev);
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_merge.c
> b/drivers/gpu/drm/mediatek/mtk_disp_merge.c
> index 470ebc4b5296..558e0cb2a297 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_merge.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_merge.c
> @@ -17,6 +17,7 @@
> #define DISP_REG_MERGE_CTRL0x000
> #define MERGE_EN 1
> #define DISP_REG_MERGE_CFG_0 0x010
> +#define DISP_REG_MERGE_CFG_1 0x014
> #define DISP_REG_MERGE_CFG_4 0x020
> #define DISP_REG_MERGE_CFG_10 0x038
> /* no swap */
> @@ -25,9 +26,12 @@
> #define DISP_REG_MERGE_CFG_12 0x040
> #define CFG_10_10_1PI_2PO_BUF_MODE 6
> #define CFG_10_10_2PI_2PO_BUF_MODE 8
> +#define CFG_11_10_1PI_2PO_MERGE18
> #define FLD_CFG_MERGE_MODE GENMASK(4, 0)
> #define DISP_REG_MERGE_CFG_24 0x070
> #define DISP_REG_MERGE_CFG_25 0x074
> +#define DISP_REG_MERGE_CFG_26 0x078
> +#define DISP_REG_MERGE_CFG_27 0x07c
> #define DISP_REG_MERGE_CFG_36 0x0a0
> #define ULTRA_EN BIT(0)
> #define PREULTRA_ENBIT(4)
> @@ -98,12 +102,19 @@ static void mtk_merge_fifo_setting(struct mtk_disp_merge
> *priv,
> void mtk_merge_config(struct device *dev, unsigned int w,
> unsigned int h, unsigned int vrefresh,
> unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
> +{
> + mtk_merge_advance_config(dev, w, 0, h, vrefresh, bpc, cmdq_pkt);
> +}
> +
> +void mtk_merge_advance_config(struct device *dev, unsigned int l_w, unsigned
> int r_w,
> + unsigned int h, unsigned int vrefresh, unsigned
> int bpc,
> + struct cmdq_pkt *cmdq_pkt)
> {
> struct mtk_disp_merge *priv = dev_get_drvdata(dev);
> unsigned int mode = CFG_10_10_1PI_2PO_BUF_MODE;
>
> - if (!h || !w) {
> - dev_err(dev, "%s: input width(%d) or height(%d) is
> invalid\n", __func__, w, h);
> + if (!h || !l_w) {
> + dev_err(dev, "%s: input width(%d) or height(%d) is
> invalid\n", __func__, l_w, h);
> return;
> }
>
> @@ -112,14 +123,41 @@ void mtk_merge_config(struct device *dev, unsigned int
> w,
> mode = CFG_10_10_2PI_2PO_BUF_MODE;
> }
>
> - mtk_ddp_write(cmdq_pkt, h << 16 | w, >cmdq_reg, priv->regs,
> + if (r_w)
> + mode = CFG_11_10_1PI_2PO_MERGE;
> +
> + mtk_ddp_write(cmdq_pkt, h << 16 | l_w, >cmdq_reg, priv->regs,
> DISP_REG_MERGE_CFG_0);
> - mtk_ddp_write(cmdq_pkt, h << 16 | w, >cmdq_reg, priv->regs,
> + mtk_ddp_write(cmdq_pkt, h << 16 | r_w, >cmdq_reg, priv->regs,
> + DISP_REG_MERGE_CFG_1);
> + mtk_ddp_write(cmdq_pkt, h << 16 | (l_w + r_w), >cmdq_reg,
> priv->regs,
> DISP_REG_MERGE_CFG_4);
> - mtk_ddp_write(cmdq_pkt, h << 16 | w, >cmdq_reg, priv->regs,
> + /*
> +* DISP_REG_MERGE_CFG_24 is merge SRAM0 w/h
> +* DISP_REG_MERGE_CFG_25 is merge SRAM1 w/h.
> +* If r_w > 0, the merge is in merge mode (input0 and input1 merge
> together),
> +* the input0 goes to SRAM0, and input1 goes to SRAM1.
> +* If r_w = 0, the merge is in buffer mode, the input goes through
> SRAM0 and
> +* then to SRAM1. Both SRAM0 and SRAM1 are set to the same size.
> +*/
> + mtk_ddp_write(cmdq_pkt, h << 16 | l_w, >cmdq_reg, priv->regs,
>
Re: [PATCH v7 12/20] drm/mediatek: add display MDP RDMA support for MT8195
Hi, Nancy: Nancy.Lin 於 2021年10月29日 週五 下午3:52寫道： > > Add MDP_RDMA driver for MT8195. MDP_RDMA is the DMA engine of > the ovl_adaptor component. Reviewed-by: Chun-Kuang Hu > > Signed-off-by: Nancy.Lin > --- > drivers/gpu/drm/mediatek/Makefile | 3 +- > drivers/gpu/drm/mediatek/mtk_disp_drv.h | 7 + > drivers/gpu/drm/mediatek/mtk_mdp_rdma.c | 316 > drivers/gpu/drm/mediatek/mtk_mdp_rdma.h | 20 ++ > 4 files changed, 345 insertions(+), 1 deletion(-) > create mode 100644 drivers/gpu/drm/mediatek/mtk_mdp_rdma.c > create mode 100644 drivers/gpu/drm/mediatek/mtk_mdp_rdma.h > > diff --git a/drivers/gpu/drm/mediatek/Makefile > b/drivers/gpu/drm/mediatek/Makefile > index a38e88e82d12..6e604a933ed0 100644 > --- a/drivers/gpu/drm/mediatek/Makefile > +++ b/drivers/gpu/drm/mediatek/Makefile > @@ -13,7 +13,8 @@ mediatek-drm-y := mtk_disp_aal.o \ > mtk_drm_gem.o \ > mtk_drm_plane.o \ > mtk_dsi.o \ > - mtk_dpi.o > + mtk_dpi.o \ > + mtk_mdp_rdma.o > > obj-$(CONFIG_DRM_MEDIATEK) += mediatek-drm.o > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h > b/drivers/gpu/drm/mediatek/mtk_disp_drv.h > index a33b13fe2b6e..b3a372cab0bd 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h > @@ -8,6 +8,7 @@ > > #include > #include "mtk_drm_plane.h" > +#include "mtk_mdp_rdma.h" > > int mtk_aal_clk_enable(struct device *dev); > void mtk_aal_clk_disable(struct device *dev); > @@ -106,4 +107,10 @@ void mtk_rdma_enable_vblank(struct device *dev, > void *vblank_cb_data); > void mtk_rdma_disable_vblank(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); > +void mtk_mdp_rdma_stop(struct device *dev, struct cmdq_pkt *cmdq_pkt); > +void mtk_mdp_rdma_config(struct device *dev, struct mtk_mdp_rdma_cfg *cfg, > +struct cmdq_pkt *cmdq_pkt); > #endif > diff --git a/drivers/gpu/drm/mediatek/mtk_mdp_rdma.c > b/drivers/gpu/drm/mediatek/mtk_mdp_rdma.c > new file mode 100644 > index ..d7926e43e77e > --- /dev/null > +++ b/drivers/gpu/drm/mediatek/mtk_mdp_rdma.c > @@ -0,0 +1,316 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2021 MediaTek Inc. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "mtk_disp_drv.h" > +#include "mtk_drm_drv.h" > +#include "mtk_mdp_rdma.h" > + > +#define MDP_RDMA_EN0x000 > +#define FLD_ROT_ENABLE BIT(0) > +#define MDP_RDMA_RESET 0x008 > +#define MDP_RDMA_CON 0x020 > +#define FLD_OUTPUT_10B BIT(5) > +#define FLD_SIMPLE_MODEBIT(4) > +#define MDP_RDMA_GMCIF_CON 0x028 > +#define FLD_COMMAND_DIVBIT(0) > +#define FLD_EXT_PREULTRA_ENBIT(3) > +#define FLD_RD_REQ_TYPEGENMASK(7, 4) > +#define VAL_RD_REQ_TYPE_BURST_8_ACCESS 7 > +#define FLD_ULTRA_EN GENMASK(13, 12) > +#define VAL_ULTRA_EN_ENABLE1 > +#define FLD_PRE_ULTRA_EN GENMASK(17, 16) > +#define VAL_PRE_ULTRA_EN_ENABLE1 > +#define FLD_EXT_ULTRA_EN BIT(18) > +#define MDP_RDMA_SRC_CON 0x030 > +#define FLD_OUTPUT_ARGBBIT(25) > +#define FLD_BIT_NUMBER GENMASK(19, 18) > +#define FLD_SWAP BIT(14) > +#define FLD_UNIFORM_CONFIG BIT(17) > +#define RDMA_INPUT_10BIT BIT(18) > +#define FLD_SRC_FORMAT GENMASK(3, 0) > +#define MDP_RDMA_COMP_CON 0x038 > +#define FLD_AFBC_ENBIT(22) > +#define FLD_AFBC_YUV_TRANSFORM BIT(21) > +#define FLD_UFBDC_EN BIT(12) > +#define MDP_RDMA_MF_BKGD_SIZE_IN_BYTE 0x060 > +#define FLD_MF_BKGD_WB GENMASK(22, 0) > +#define MDP_RDMA_MF_SRC_SIZE 0x070 > +#define FLD_MF_SRC_H GENMASK(30, 16) > +#define FLD_MF_SRC_W GENMASK(14, 0) > +#define MDP_RDMA_MF_CLIP_SIZE 0x078 > +#define FLD_MF_CLIP_H GENMASK(30, 16) > +#define FLD_MF_CLIP_W GENMASK(14, 0) > +#define MDP_RDMA_SRC_OFFSET_0 0x118 > +#define FLD_SRC_OFFSET_0 GENMASK(31, 0) > +#define MDP_RDMA_TRANSFORM_0 0x200 > +#define FLD_INT_MATRIX_SEL GENMASK(27, 23) > +#define FLD_TRANS_EN
[PATCH v2 3/3] backlight: lp855x: Add support ACPI enumeration
The Xiaomi Mi Pad 2 tablet uses an ACPI enumerated LP8556 backlight
controller for its LCD-panel, with a Xiaomi specific ACPI HID of
"XMCC0001", add support for this.
Note the new "if (id)" check also fixes a NULL pointer deref when a user
tries to manually bind the driver from sysfs.
When CONFIG_ACPI is disabled acpi_match_device() will always return NULL,
so the lp855x_parse_acpi() call will get optimized away.
Signed-off-by: Hans de Goede
---
Changes in v2:
- Remove `lp->pdata = pdata` assignment from lp855x_parse_dt()
- Add "and is in register mode" to the comment in
lp855x_parse_acpi()
---
drivers/video/backlight/lp855x_bl.c | 73 -
1 file changed, 61 insertions(+), 12 deletions(-)
diff --git a/drivers/video/backlight/lp855x_bl.c
b/drivers/video/backlight/lp855x_bl.c
index d1d27d5eb0f2..2b9e203e 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -5,6 +5,7 @@
* Copyright (C) 2011 Texas Instruments
*/
+#include
#include
#include
#include
@@ -330,7 +331,7 @@ static int lp855x_parse_dt(struct lp855x *lp)
{
struct device *dev = lp->dev;
struct device_node *node = dev->of_node;
- struct lp855x_platform_data *pdata;
+ struct lp855x_platform_data *pdata = lp->pdata;
int rom_length;
if (!node) {
@@ -338,10 +339,6 @@ static int lp855x_parse_dt(struct lp855x *lp)
return -EINVAL;
}
- pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
- if (!pdata)
- return -ENOMEM;
-
of_property_read_string(node, "bl-name", >name);
of_property_read_u8(node, "dev-ctrl", >device_control);
of_property_read_u8(node, "init-brt", >initial_brightness);
@@ -368,8 +365,6 @@ static int lp855x_parse_dt(struct lp855x *lp)
pdata->rom_data = [0];
}
- lp->pdata = pdata;
-
return 0;
}
#else
@@ -379,8 +374,32 @@ static int lp855x_parse_dt(struct lp855x *lp)
}
#endif
+static int lp855x_parse_acpi(struct lp855x *lp)
+{
+ int ret;
+
+ /*
+* On ACPI the device has already been initialized by the firmware
+* and is in register mode, so we can read back the settings from
+* the registers.
+*/
+ ret = i2c_smbus_read_byte_data(lp->client, lp->cfg->reg_brightness);
+ if (ret < 0)
+ return ret;
+
+ lp->pdata->initial_brightness = ret;
+
+ ret = i2c_smbus_read_byte_data(lp->client, lp->cfg->reg_devicectrl);
+ if (ret < 0)
+ return ret;
+
+ lp->pdata->device_control = ret;
+ return 0;
+}
+
static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
{
+ const struct acpi_device_id *acpi_id = NULL;
struct device *dev = >dev;
struct lp855x *lp;
int ret;
@@ -394,10 +413,20 @@ static int lp855x_probe(struct i2c_client *cl, const
struct i2c_device_id *id)
lp->client = cl;
lp->dev = dev;
- lp->chipname = id->name;
- lp->chip_id = id->driver_data;
lp->pdata = dev_get_platdata(dev);
+ if (id) {
+ lp->chipname = id->name;
+ lp->chip_id = id->driver_data;
+ } else {
+ acpi_id = acpi_match_device(dev->driver->acpi_match_table, dev);
+ if (!acpi_id)
+ return -ENODEV;
+
+ lp->chipname = acpi_id->id;
+ lp->chip_id = acpi_id->driver_data;
+ }
+
switch (lp->chip_id) {
case LP8550:
case LP8551:
@@ -415,9 +444,19 @@ static int lp855x_probe(struct i2c_client *cl, const
struct i2c_device_id *id)
}
if (!lp->pdata) {
- ret = lp855x_parse_dt(lp);
- if (ret < 0)
- return ret;
+ lp->pdata = devm_kzalloc(dev, sizeof(*lp->pdata), GFP_KERNEL);
+ if (!lp->pdata)
+ return -ENOMEM;
+
+ if (id) {
+ ret = lp855x_parse_dt(lp);
+ if (ret < 0)
+ return ret;
+ } else {
+ ret = lp855x_parse_acpi(lp);
+ if (ret < 0)
+ return ret;
+ }
}
if (lp->pdata->period_ns > 0)
@@ -537,10 +576,20 @@ static const struct i2c_device_id lp855x_ids[] = {
};
MODULE_DEVICE_TABLE(i2c, lp855x_ids);
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id lp855x_acpi_match[] = {
+ /* Xiaomi specific HID used for the LP8556 on the Mi Pad 2 */
+ { "XMCC0001", LP8556 },
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, lp855x_acpi_match);
+#endif
+
static struct i2c_driver lp855x_driver = {
.driver = {
.name = "lp855x",
.of_match_table = of_match_ptr(lp855x_dt_ids),
+ .acpi_match_table =
[PATCH v2 1/3] backlight: lp855x: Move device_config setting out of lp855x_configure()
Move the setting of the lp->cfg pointer to the chip specific
lp855x_device_config struct from lp855x_configure() to
lp855x_probe(), before calling lp855x_parse_dt().
This is a preperation patch for adding ACPI enumeration support.
Reviewed-by: Daniel Thompson
Signed-off-by: Hans de Goede
---
drivers/video/backlight/lp855x_bl.c | 32 ++---
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/video/backlight/lp855x_bl.c
b/drivers/video/backlight/lp855x_bl.c
index e94932c69f54..808ff00b2003 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -170,22 +170,6 @@ static int lp855x_configure(struct lp855x *lp)
int i, ret;
struct lp855x_platform_data *pd = lp->pdata;
- switch (lp->chip_id) {
- case LP8550:
- case LP8551:
- case LP8552:
- case LP8553:
- case LP8556:
- lp->cfg = _dev_cfg;
- break;
- case LP8555:
- case LP8557:
- lp->cfg = _dev_cfg;
- break;
- default:
- return -EINVAL;
- }
-
if (lp->cfg->pre_init_device) {
ret = lp->cfg->pre_init_device(lp);
if (ret) {
@@ -413,6 +397,22 @@ static int lp855x_probe(struct i2c_client *cl, const
struct i2c_device_id *id)
lp->chip_id = id->driver_data;
lp->pdata = dev_get_platdata(>dev);
+ switch (lp->chip_id) {
+ case LP8550:
+ case LP8551:
+ case LP8552:
+ case LP8553:
+ case LP8556:
+ lp->cfg = _dev_cfg;
+ break;
+ case LP8555:
+ case LP8557:
+ lp->cfg = _dev_cfg;
+ break;
+ default:
+ return -EINVAL;
+ }
+
if (!lp->pdata) {
ret = lp855x_parse_dt(lp);
if (ret < 0)
--
2.31.1
[PATCH v2 2/3] backlight: lp855x: Add dev helper variable to lp855x_probe()
Add a dev local variable to the lp855x_probe(), to replace ">dev"
and "lp->dev" in various places.
Also switch to dev_err_probe() in one case which takes care of not
printing -EPROBE_DEFER errors for us.
This is mostly a preparation for adding ACPI enumeration support which
will use the new "dev" variable more.
Reviewed-by: Daniel Thompson
Signed-off-by: Hans de Goede
---
drivers/video/backlight/lp855x_bl.c | 29 +
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/video/backlight/lp855x_bl.c
b/drivers/video/backlight/lp855x_bl.c
index 808ff00b2003..d1d27d5eb0f2 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -381,21 +381,22 @@ static int lp855x_parse_dt(struct lp855x *lp)
static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
{
+ struct device *dev = >dev;
struct lp855x *lp;
int ret;
if (!i2c_check_functionality(cl->adapter, I2C_FUNC_SMBUS_I2C_BLOCK))
return -EIO;
- lp = devm_kzalloc(>dev, sizeof(struct lp855x), GFP_KERNEL);
+ lp = devm_kzalloc(dev, sizeof(struct lp855x), GFP_KERNEL);
if (!lp)
return -ENOMEM;
lp->client = cl;
- lp->dev = >dev;
+ lp->dev = dev;
lp->chipname = id->name;
lp->chip_id = id->driver_data;
- lp->pdata = dev_get_platdata(>dev);
+ lp->pdata = dev_get_platdata(dev);
switch (lp->chip_id) {
case LP8550:
@@ -424,30 +425,27 @@ static int lp855x_probe(struct i2c_client *cl, const
struct i2c_device_id *id)
else
lp->mode = REGISTER_BASED;
- lp->supply = devm_regulator_get(lp->dev, "power");
+ lp->supply = devm_regulator_get(dev, "power");
if (IS_ERR(lp->supply)) {
if (PTR_ERR(lp->supply) == -EPROBE_DEFER)
return -EPROBE_DEFER;
lp->supply = NULL;
}
- lp->enable = devm_regulator_get_optional(lp->dev, "enable");
+ lp->enable = devm_regulator_get_optional(dev, "enable");
if (IS_ERR(lp->enable)) {
ret = PTR_ERR(lp->enable);
if (ret == -ENODEV) {
lp->enable = NULL;
} else {
- if (ret != -EPROBE_DEFER)
- dev_err(lp->dev, "error getting enable
regulator: %d\n",
- ret);
- return ret;
+ return dev_err_probe(dev, ret, "getting enable
regulator\n");
}
}
if (lp->supply) {
ret = regulator_enable(lp->supply);
if (ret < 0) {
- dev_err(>dev, "failed to enable supply: %d\n", ret);
+ dev_err(dev, "failed to enable supply: %d\n", ret);
return ret;
}
}
@@ -455,7 +453,7 @@ static int lp855x_probe(struct i2c_client *cl, const struct
i2c_device_id *id)
if (lp->enable) {
ret = regulator_enable(lp->enable);
if (ret < 0) {
- dev_err(lp->dev, "failed to enable vddio: %d\n", ret);
+ dev_err(dev, "failed to enable vddio: %d\n", ret);
goto disable_supply;
}
@@ -470,20 +468,19 @@ static int lp855x_probe(struct i2c_client *cl, const
struct i2c_device_id *id)
ret = lp855x_configure(lp);
if (ret) {
- dev_err(lp->dev, "device config err: %d", ret);
+ dev_err(dev, "device config err: %d", ret);
goto disable_vddio;
}
ret = lp855x_backlight_register(lp);
if (ret) {
- dev_err(lp->dev,
- "failed to register backlight. err: %d\n", ret);
+ dev_err(dev, "failed to register backlight. err: %d\n", ret);
goto disable_vddio;
}
- ret = sysfs_create_group(>dev->kobj, _attr_group);
+ ret = sysfs_create_group(>kobj, _attr_group);
if (ret) {
- dev_err(lp->dev, "failed to register sysfs. err: %d\n", ret);
+ dev_err(dev, "failed to register sysfs. err: %d\n", ret);
goto disable_vddio;
}
--
2.31.1
Re: [PATCH 3/3] backlight: lp855x: Add support ACPI enumeration
Hi Daniel,
Thank you for the quick review of this series.
On 11/2/21 13:22, Daniel Thompson wrote:
> On Mon, Nov 01, 2021 at 07:55:17PM +0100, Hans de Goede wrote:
>> The Xiaomi Mi Pad 2 tablet uses an ACPI enumerated LP8556 backlight
>> controller for its LCD-panel, with a Xiaomi specific ACPI HID of
>> "XMCC0001", add support for this.
>>
>> Note the new "if (id)" check also fixes a NULL pointer deref when a user
>> tries to manually bind the driver from sysfs.
>>
>> When CONFIG_ACPI is disabled acpi_match_device() will always return NULL,
>> so the lp855x_parse_acpi() call will get optimized away.
>>
>> Signed-off-by: Hans de Goede
>> ---
>> drivers/video/backlight/lp855x_bl.c | 70 -
>> 1 file changed, 60 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/video/backlight/lp855x_bl.c
>> b/drivers/video/backlight/lp855x_bl.c
>> index d1d27d5eb0f2..f075ec84acfb 100644
>> --- a/drivers/video/backlight/lp855x_bl.c
>> +++ b/drivers/video/backlight/lp855x_bl.c
>> @@ -338,10 +339,6 @@ static int lp855x_parse_dt(struct lp855x *lp)
>> return -EINVAL;
>> }
>>
>> -pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>> -if (!pdata)
>> -return -ENOMEM;
>> -
>> of_property_read_string(node, "bl-name", >name);
>> of_property_read_u8(node, "dev-ctrl", >device_control);
>> of_property_read_u8(node, "init-brt", >initial_brightness);
>
> Shouldn't there be a removal of an `lp->pdata = pdata` from somewhere in
> this function?
Ack, fixed for v2.
>> @@ -379,8 +376,31 @@ static int lp855x_parse_dt(struct lp855x *lp)
>> }
>> #endif
>>
>> +static int lp855x_parse_acpi(struct lp855x *lp)
>> +{
>> +int ret;
>> +
>> +/*
>> + * On ACPI the device has already been initialized by the firmware
>
> Perhaps nitpicking but ideally I'd like "and is in register mode" here
> since I presume it can also be assumed that everything with this HID
> has adopted that).
Nope not nitpicking, that is a good point, also fixed for v2.
Regards,
Hans
Re: [PATCH 2/4] drm/dp_mst: Only create connector for connected end device
On Fri, 2021-10-29 at 12:11 +, Lin, Wayne wrote:
> [Public]
>
> Thanks Lyude for patiently guiding on this : )
> Would like to learn more as following
I do follow your bit about connectors only being created when a virtual path
is instantiated, but that still doesn't follow how connectors in DRM typically
behave though as this idea still comes down to "we don't have disconnected
connectors, only connected ones". Which still breaks force probing (if the
connector doesn't exist in userspace because we destroyed it, how do we get to
it's force sysfs file?), and also just makes hides information from userspace
that it might actually care about (what if for instance, a GUI wanted to display
the topology layout of an MST hub -including- all of the currently disconnected
ports on it? Considering we allow this for things like USB, it doesn't make
sense to hide them for MST.
As well, while your idea for what an MST connector is honestly does make a lot
more sense then what we have, that's not really the issue here. The problem is
that connector creation/destruction is already quite racy, and requires a _lot_
of care to get right. We've already had tons of bugs in the past that lead to us
resorting to all of the tricks we're currently using, for instance:
Which just seems to add a lot of
complication to the current MST code, without much reason here besides trying
to reduce the amount of connectors along with a potential bug with leaking
connectors that we still don't know the cause of. Trying to solve problems
without understanding exactly what's causing them
something around a bug that could be entirely unrelated to how we create
connectors, because then it's not even really guaranteed we've fixed anything
if we don't know what caused the problem in the first place. Working around
problems might temporarily fix the ones we're dealing with right now, but
without understanding what's causing it there's no guarantee it won't just pop
up again in the future or that we won't introduce new problems in the process.
>
> >
> > Regardless though, I would think that we could just handle this mostly
> > from the atomic state even with a connector for every port. For
> > instance, i915 already has something called "big joiner" for supporting
> > display configurations where one display can take up two
> > separate display pipes (CRTCs). We could likely do something similar but
> > with connectors if we end up having to deal with a display
> > driven by two DP links.
> >
> > > I was thinking to associate a drm connector for end stream sink only.
> > > I think we probably won't want to attach a connector to a
> > > relay/retimer/redriver within a stream path? I treat MST port as the
> > > similar role when It's fixed to connect to a MST branch device.
> >
> > If it's a fixed connection, this might actually be OK to avoid attaching
> > connectors on. Currently with input ports where we know we can
> > never receive a CSN for them during runtime, we're able to avoid creating
> > a connector because no potential for CSN during runtime
> > means the only possible time an input port could transition would be
> > suspend/resume. So if we detect we're on another topology
> > where something that was previously an output port that is an input port
> > on the new topology, we get rid of the connector by
> > removing the drm_dp_mst_port it's associated with from the topology and
> > replace it with a new one. This works pretty well, as it
> > avoids doing any actual connector destruction from the suspend/resume
> > codepath and ensures that any pointer references to the
> > now non-existent output port remain valid for as long as needed. So I
> > might actually be open to expanding this for fixed connections
> > like relays, retimers and redrivers if we handle things in a similar
> > manner.
> > For anything that can receive a CSN though, a drm_connector is
> > unconditionally needed even if nothing's connected.
>
> Want to deepen my knowledge here. Sorry Lyude could you explain more on this
> please?
> Are you saying that if we change to associate drm connector as what I
> proposed in this patch, we will create actual connector destruction
> from the suspend/resume codepath and which is a problem here? I thought once
> the connection status changed from connected to
> disconnected during suspend/resume, we still use the same way as what we did
> in drm_dp_delayed_destroy_port():
> i.e.
> if (port->connector) {
> drm_connector_unregister(port->connector);
> drm_connector_put(port->connector);
> }
> We won't directly destruct the drm connector?
Something like that, I'd need to to go look further into the details because I
very vividly remember most of the tricks we do in the MST helpers regarding
delayed connector destruction and when/how we change various members of the
drm_dp_mst_port/drm_dp_mst_branch structures. I vaguely remember the problem
with trying to hot add/remove connectors (I -did- actually try
[PATCH 0/3] i915: Initial workarounds for Xe_HP SDV and DG2
This is the initial batch of workarounds for these two platforms. There are still more workarounds to come in the future (e.g., related to other functionality that hasn't landed yet like compute engines, multi-tile, etc.). Matt Roper (2): drm/i915/dg2: Add initial gt/ctx/engine workarounds drm/i915/dg2: Program recommended HW settings Stuart Summers (1): drm/i915/xehpsdv: Add initial workarounds drivers/gpu/drm/i915/gt/intel_workarounds.c | 392 +++- drivers/gpu/drm/i915/i915_reg.h | 154 +++- drivers/gpu/drm/i915/intel_pm.c | 31 +- 3 files changed, 547 insertions(+), 30 deletions(-) -- 2.33.0
[PATCH 2/3] drm/i915/dg2: Add initial gt/ctx/engine workarounds
Bspec: 54077,68173,54833
Cc: Anusha Srivatsa
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/gt/intel_workarounds.c | 278 +++-
drivers/gpu/drm/i915/i915_reg.h | 94 +--
drivers/gpu/drm/i915/intel_pm.c | 21 +-
3 files changed, 372 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 4aaa210fc003..37fd541a9719 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -644,6 +644,42 @@ static void dg1_ctx_workarounds_init(struct
intel_engine_cs *engine,
DG1_HZ_READ_SUPPRESSION_OPTIMIZATION_DISABLE);
}
+static void dg2_ctx_workarounds_init(struct intel_engine_cs *engine,
+struct i915_wa_list *wal)
+{
+ gen12_ctx_gt_tuning_init(engine, wal);
+
+ /* Wa_16011186671:dg2_g11 */
+ if (IS_DG2_GRAPHICS_STEP(engine->i915, G11, STEP_A0, STEP_B0)) {
+ wa_masked_dis(wal, VFLSKPD, DIS_MULT_MISS_RD_SQUASH);
+ wa_masked_en(wal, VFLSKPD, DIS_OVER_FETCH_CACHE);
+ }
+
+ if (IS_DG2_GRAPHICS_STEP(engine->i915, G10, STEP_A0, STEP_B0)) {
+ /* Wa_14010469329:dg2_g10 */
+ wa_masked_en(wal, GEN11_COMMON_SLICE_CHICKEN3,
+XEHP_DUAL_SIMD8_SEQ_MERGE_DISABLE);
+
+ /*
+* Wa_22010465075:dg2_g10
+* Wa_22010613112:dg2_g10
+* Wa_14010698770:dg2_g10
+*/
+ wa_masked_en(wal, GEN11_COMMON_SLICE_CHICKEN3,
+GEN12_DISABLE_CPS_AWARE_COLOR_PIPE);
+ }
+
+ /* Wa_16013271637:dg2 */
+ wa_masked_en(wal, SLICE_COMMON_ECO_CHICKEN1,
+MSC_MSAA_REODER_BUF_BYPASS_DISABLE);
+
+ /* Wa_22012532006:dg2 */
+ if (IS_DG2_GRAPHICS_STEP(engine->i915, G10, STEP_A0, STEP_C0) ||
+ IS_DG2_GRAPHICS_STEP(engine->i915, G11, STEP_A0, STEP_B0))
+ wa_masked_en(wal, GEN9_HALF_SLICE_CHICKEN7,
+DG2_DISABLE_ROUND_ENABLE_ALLOW_FOR_SSLA);
+}
+
static void fakewa_disable_nestedbb_mode(struct intel_engine_cs *engine,
struct i915_wa_list *wal)
{
@@ -730,7 +766,9 @@ __intel_engine_init_ctx_wa(struct intel_engine_cs *engine,
if (engine->class != RENDER_CLASS)
goto done;
- if (IS_XEHPSDV(i915))
+ if (IS_DG2(i915))
+ dg2_ctx_workarounds_init(engine, wal);
+ else if (IS_XEHPSDV(i915))
; /* noop; none at this time */
else if (IS_DG1(i915))
dg1_ctx_workarounds_init(engine, wal);
@@ -1343,12 +1381,117 @@ xehpsdv_gt_workarounds_init(struct intel_gt *gt,
struct i915_wa_list *wal)
GLOBAL_INVALIDATION_MODE);
}
+static void
+dg2_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
+{
+ struct intel_engine_cs *engine;
+ int id;
+
+ xehp_init_mcr(gt, wal);
+
+ /* Wa_14011060649:dg2 */
+ wa_14011060649(gt, wal);
+
+ /*
+* Although there are per-engine instances of these registers,
+* they technically exist outside the engine itself and are not
+* impacted by engine resets. Furthermore, they're part of the
+* GuC blacklist so trying to treat them as engine workarounds
+* will result in GuC initialization failure and a wedged GPU.
+*/
+ for_each_engine(engine, gt, id) {
+ if (engine->class != VIDEO_DECODE_CLASS)
+ continue;
+
+ /* Wa_16010515920:dg2_g10 */
+ if (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_B0))
+ wa_write_or(wal, VDBOX_CGCTL3F18(engine->mmio_base),
+ ALNUNIT_CLKGATE_DIS);
+ }
+
+ if (IS_DG2_G10(gt->i915)) {
+ /* Wa_22010523718:dg2 */
+ wa_write_or(wal, UNSLICE_UNIT_LEVEL_CLKGATE,
+ CG3DDISCFEG_CLKGATE_DIS);
+
+ /* Wa_14011006942:dg2 */
+ wa_write_or(wal, SUBSLICE_UNIT_LEVEL_CLKGATE,
+ DSS_ROUTER_CLKGATE_DIS);
+ }
+
+ if (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_B0)) {
+ /* Wa_14010680813:dg2_g10 */
+ wa_write_or(wal, GEN12_GAMSTLB_CTRL, CONTROL_BLOCK_CLKGATE_DIS |
+ EGRESS_BLOCK_CLKGATE_DIS | TAG_BLOCK_CLKGATE_DIS);
+
+ /* Wa_14010948348:dg2_g10 */
+ wa_write_or(wal, UNSLCGCTL9430, MSQDUNIT_CLKGATE_DIS);
+
+ /* Wa_14011037102:dg2_g10 */
+ wa_write_or(wal, UNSLCGCTL9444, LTCDD_CLKGATE_DIS);
+
+ /* Wa_14011371254:dg2_g10 */
+ wa_write_or(wal, SLICE_UNIT_LEVEL_CLKGATE, NODEDSS_CLKGATE_DIS);
+
+ /* Wa_14011431319:dg2_g10 */
+
[PATCH 1/3] drm/i915/xehpsdv: Add initial workarounds
From: Stuart Summers
Add the initial set of workarounds for Xe_HP SDV.
There are some additional workarounds specific to the compute engines
that we're holding back for now. Those will be added later, after
general compute engine support lands.
Cc: Lucas De Marchi
Signed-off-by: Stuart Summers
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/gt/intel_workarounds.c | 94 ++---
drivers/gpu/drm/i915/i915_reg.h | 53
drivers/gpu/drm/i915/intel_pm.c | 12 ++-
3 files changed, 146 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 45936f624a1e..4aaa210fc003 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -730,7 +730,9 @@ __intel_engine_init_ctx_wa(struct intel_engine_cs *engine,
if (engine->class != RENDER_CLASS)
goto done;
- if (IS_DG1(i915))
+ if (IS_XEHPSDV(i915))
+ ; /* noop; none at this time */
+ else if (IS_DG1(i915))
dg1_ctx_workarounds_init(engine, wal);
else if (GRAPHICS_VER(i915) == 12)
gen12_ctx_workarounds_init(engine, wal);
@@ -1277,7 +1279,68 @@ dg1_gt_workarounds_init(struct intel_gt *gt, struct
i915_wa_list *wal)
static void
xehpsdv_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
{
+ struct drm_i915_private *i915 = gt->i915;
+
xehp_init_mcr(gt, wal);
+
+ /* Wa_1409757795:xehpsdv */
+ wa_write_or(wal, SCCGCTL94DC, CG3DDISURB);
+
+ /* Wa_18011725039:xehpsdv */
+ if (IS_XEHPSDV_GRAPHICS_STEP(i915, STEP_A1, STEP_B0)) {
+ wa_masked_dis(wal, MLTICTXCTL, TDONRENDER);
+ wa_write_or(wal, L3SQCREG1_CCS0, FLUSHALLNONCOH);
+ }
+
+ /* Wa_16011155590:xehpsdv */
+ if (IS_XEHPSDV_GRAPHICS_STEP(i915, STEP_A0, STEP_B0))
+ wa_write_or(wal, UNSLICE_UNIT_LEVEL_CLKGATE,
+ TSGUNIT_CLKGATE_DIS);
+
+ /* Wa_14011780169:xehpsdv */
+ if (IS_XEHPSDV_GRAPHICS_STEP(i915, STEP_B0, STEP_FOREVER)) {
+ wa_write_or(wal, UNSLCGCTL9440, GAMTLBOACS_CLKGATE_DIS |
+ GAMTLBVDBOX7_CLKGATE_DIS |
+ GAMTLBVDBOX6_CLKGATE_DIS |
+ GAMTLBVDBOX5_CLKGATE_DIS |
+ GAMTLBVDBOX4_CLKGATE_DIS |
+ GAMTLBVDBOX3_CLKGATE_DIS |
+ GAMTLBVDBOX2_CLKGATE_DIS |
+ GAMTLBVDBOX1_CLKGATE_DIS |
+ GAMTLBVDBOX0_CLKGATE_DIS |
+ GAMTLBKCR_CLKGATE_DIS |
+ GAMTLBGUC_CLKGATE_DIS |
+ GAMTLBBLT_CLKGATE_DIS);
+ wa_write_or(wal, UNSLCGCTL9444, GAMTLBGFXA0_CLKGATE_DIS |
+ GAMTLBGFXA1_CLKGATE_DIS |
+ GAMTLBCOMPA0_CLKGATE_DIS |
+ GAMTLBCOMPA1_CLKGATE_DIS |
+ GAMTLBCOMPB0_CLKGATE_DIS |
+ GAMTLBCOMPB1_CLKGATE_DIS |
+ GAMTLBCOMPC0_CLKGATE_DIS |
+ GAMTLBCOMPC1_CLKGATE_DIS |
+ GAMTLBCOMPD0_CLKGATE_DIS |
+ GAMTLBCOMPD1_CLKGATE_DIS |
+ GAMTLBMERT_CLKGATE_DIS |
+ GAMTLBVEBOX3_CLKGATE_DIS |
+ GAMTLBVEBOX2_CLKGATE_DIS |
+ GAMTLBVEBOX1_CLKGATE_DIS |
+ GAMTLBVEBOX0_CLKGATE_DIS);
+ }
+
+ /* Wa_14012362059:xehpsdv */
+ wa_write_or(wal, GEN12_MERT_MOD_CTRL, FORCE_MISS_FTLB);
+
+ /* Wa_16012725990:xehpsdv */
+ if (IS_XEHPSDV_GRAPHICS_STEP(i915, STEP_A1, STEP_FOREVER))
+ wa_write_or(wal, UNSLICE_UNIT_LEVEL_CLKGATE,
VFUNIT_CLKGATE_DIS);
+
+ /* Wa_14011060649:xehpsdv */
+ wa_14011060649(gt, wal);
+
+ /* Wa_14014368820:xehpsdv */
+ wa_write_or(wal, GEN12_GAMCNTRL_CTRL, INVALIDATION_BROADCAST_MODE_DIS |
+ GLOBAL_INVALIDATION_MODE);
}
static void
@@ -1559,7 +1622,7 @@ static void cfl_whitelist_build(struct intel_engine_cs
*engine)
RING_FORCE_TO_NONPRIV_RANGE_4);
}
-static void cml_whitelist_build(struct intel_engine_cs *engine)
+static void allow_read_ctx_timestamp(struct intel_engine_cs *engine)
{
struct i915_wa_list *w = >whitelist;
@@ -1567,6 +1630,11 @@ static void cml_whitelist_build(struct intel_engine_cs
*engine)
whitelist_reg_ext(w,
RING_CTX_TIMESTAMP(engine->mmio_base),
RING_FORCE_TO_NONPRIV_ACCESS_RD);
+}
+
+static void cml_whitelist_build(struct intel_engine_cs *engine)
+{
+ allow_read_ctx_timestamp(engine);
[PATCH 3/3] drm/i915/dg2: Program recommended HW settings
The bspec's performance guide suggests programming specific values into
a few registers for optimal performance. Although these aren't
workarounds, it's easiest to handle them inside the GT workaround
functions (which will also ensure that the values set here are properly
melded with other bits in the same registers that _are_ set by
workarounds).
Bspec: 68331, 45395
Cc: Matt Atwood
Cc: Lucas De Marchi
Cc: Siddiqui Ayaz A
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/gt/intel_workarounds.c | 26 -
drivers/gpu/drm/i915/i915_reg.h | 9 +++
2 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 37fd541a9719..51591119da15 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -558,6 +558,22 @@ static void icl_ctx_workarounds_init(struct
intel_engine_cs *engine,
wa_masked_en(wal, GEN9_ROW_CHICKEN4, GEN11_DIS_PICK_2ND_EU);
}
+/*
+ * These settings aren't actually workarounds, but general tuning settings that
+ * need to be programmed on dg2 platform.
+ */
+static void dg2_ctx_gt_tuning_init(struct intel_engine_cs *engine,
+ struct i915_wa_list *wal)
+{
+ wa_write_clr_set(wal, GEN11_L3SQCREG5, L3_PWM_TIMER_INIT_VAL_MASK,
+REG_FIELD_PREP(L3_PWM_TIMER_INIT_VAL_MASK, 0x7f));
+ wa_add(wal,
+ FF_MODE2,
+ FF_MODE2_TDS_TIMER_MASK,
+ FF_MODE2_TDS_TIMER_128,
+ 0, false);
+}
+
/*
* These settings aren't actually workarounds, but general tuning settings that
* need to be programmed on several platforms.
@@ -647,7 +663,7 @@ static void dg1_ctx_workarounds_init(struct intel_engine_cs
*engine,
static void dg2_ctx_workarounds_init(struct intel_engine_cs *engine,
struct i915_wa_list *wal)
{
- gen12_ctx_gt_tuning_init(engine, wal);
+ dg2_ctx_gt_tuning_init(engine, wal);
/* Wa_16011186671:dg2_g11 */
if (IS_DG2_GRAPHICS_STEP(engine->i915, G11, STEP_A0, STEP_B0)) {
@@ -1482,6 +1498,14 @@ dg2_gt_workarounds_init(struct intel_gt *gt, struct
i915_wa_list *wal)
/* Wa_14014830051:dg2 */
wa_write_clr(wal, SARB_CHICKEN1, COMP_CKN_IN);
+
+ /*
+* The following are not actually "workarounds" but rather
+* recommended tuning settings documented in the bspec's
+* performance guide section.
+*/
+ wa_write_or(wal, XEHP_L3SCQREG7, BLEND_FILL_CACHING_OPT_DIS);
+ wa_write_or(wal, GEN12_SQCM, EN_32B_ACCESS);
}
static void
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ee39d6bd0f3c..ef3b5732faad 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -731,6 +731,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define GEN12_OA_TLB_INV_CR _MMIO(0xceec)
+#define GEN12_SQCM _MMIO(0x8724)
+#define EN_32B_ACCESSREG_BIT(30)
+
/* Gen12 OAR unit */
#define GEN12_OAR_OACONTROL _MMIO(0x2960)
#define GEN12_OAR_OACONTROL_COUNTER_FORMAT_SHIFT 1
@@ -8506,6 +8509,12 @@ enum {
#define GEN8_LQSC_FLUSH_COHERENT_LINES(1 << 21)
#define GEN8_LQSQ_NONIA_COHERENT_ATOMICS_ENABLE REG_BIT(22)
+#define GEN11_L3SQCREG5_MMIO(0xb158)
+#define L3_PWM_TIMER_INIT_VAL_MASK REG_GENMASK(9, 0)
+
+#define XEHP_L3SCQREG7 _MMIO(0xb188)
+#define BLEND_FILL_CACHING_OPT_DIS REG_BIT(3)
+
/* GEN8 chicken */
#define HDC_CHICKEN0 _MMIO(0x7300)
#define ICL_HDC_MODE _MMIO(0xE5F4)
--
2.33.0
[PATCH] drm/msm/dp: employ bridge mechanism for display enable and disable
Current display mode_set, enable and disable functions are implemented
as function called directly from drm encoder. This patch have display
mode_set, enable and disable be implemented as callback function of drm
bridge.
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 21 --
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 8 ++
drivers/gpu/drm/msm/dp/dp_display.c | 26 +++
drivers/gpu/drm/msm/dp/dp_display.h | 8 ++
drivers/gpu/drm/msm/dp/dp_drm.c | 113
drivers/gpu/drm/msm/msm_drv.h | 29 ++-
6 files changed, 147 insertions(+), 58 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/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 27d98b5..3bbd09c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -557,6 +557,14 @@ static int _dpu_kms_initialize_displayport(struct
drm_device *dev,
encoder->base.id, rc);
return rc;
}
+
+ rc = msm_dp_bridge_init(priv->dp[i], dev, encoder);
+if (rc) {
+ DPU_ERROR("failed to setup DPU bridge %d: rc:%d\n",
+ encoder->base.id, rc);
+ return rc;
+}
+
}
return rc;
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index e41dd40..317f963 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -569,8 +569,8 @@ static int dp_hpd_plug_handle(struct dp_display_private
*dp, u32 data)
return 0;
};
-static int dp_display_enable(struct dp_display_private *dp, u32 data);
-static int dp_display_disable(struct dp_display_private *dp, u32 data);
+static int __dp_display_enable(struct dp_display_private *dp, u32 data);
+static int __dp_display_disable(struct dp_display_private *dp, u32 data);
static int dp_connect_pending_timeout(struct dp_display_private *dp, u32 data)
{
@@ -855,7 +855,7 @@ static int dp_display_prepare(struct msm_dp *dp_display)
return 0;
}
-static int dp_display_enable(struct dp_display_private *dp, u32 data)
+static int __dp_display_enable(struct dp_display_private *dp, u32 data)
{
int rc = 0;
@@ -898,7 +898,7 @@ static int dp_display_post_enable(struct msm_dp *dp_display)
return 0;
}
-static int dp_display_disable(struct dp_display_private *dp, u32 data)
+static int __dp_display_disable(struct dp_display_private *dp, u32 data)
{
struct msm_dp *dp_display = >dp_display;
@@ -1533,7 +1533,7 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct
drm_device *dev,
return 0;
}
-int msm_dp_display_enable(struct msm_dp *dp, struct
Re: [PATCH] backlight: led_bl: Add support for an "enable" GPIO
Le Tue, Nov 02, 2021 at 11:25:14AM +, Daniel Thompson a écrit : > On Tue, Nov 02, 2021 at 11:19:42AM +, Daniel Thompson wrote: > > On Tue, Nov 02, 2021 at 10:04:55AM +, Corentin LABBE wrote: > > > From: Jean-Jacques Hiblot > > > > > > This patch adds support for an "enable GPIO". > > > > Before taking this kind of change is there a good reason why backlight > > should manage the GPIO? I thought that the LED subsystem was a sub > > system for LEDs (not LED controllers). In other words if you direct > > that the LED be lit up then isn't it the LED driver's job to manage > > the GPIO and ensure that it lights up? > > Sorry... I should have paid more attention to my sense of déjà vu with > this patch. > > This approach was discussed and rejected when we first introduced the > led_bl driver: > https://lore.kernel.org/linux-leds/20190705100851.zn2jkipj4fxq5we6@devuan/ > Hello I am sorry, I didnt checked if the patch was already submitted or not. Regards
Re: [Intel-gfx] [PATCH v3 05/10] drm/i915: Prepare for multiple gts
Hi Tvrtko, > > > > [...] > > > > > > > > >static int > > > > >intel_gt_tile_setup(struct intel_gt *gt, unsigned int id, > > > > > phys_addr_t phys_addr) > > > > > > > > we don't actually need 'id', it's gt->info.id. It's introduced in > > > > patch 3 with the value '0' but it's not needed. > > > > > > I have a suspicion code got munged up over endless rebases and refactors. > > > > > > This patch is the one which introduces the id member to gt->info. But it > > > is not setting it, even though I suspect the intent was for > > > intel_gt_tile_setup to do that. > > > > > > Instead gt->info.id is only set to a valid value in last patch of this > > > series inside intel_gt_probe_all: > > > > > > + gt->i915 = i915; > > > + gt->name = gtdef->name; > > > + gt->type = gtdef->type; > > > + gt->info.engine_mask = gtdef->engine_mask; > > > + gt->info.id = i; > > > + > > > + drm_dbg(>drm, "Setting up %s %u\n", gt->name, > > > gt->info.id); > > > + ret = intel_gt_tile_setup(gt, i, phys_addr + > > > gtdef->mapping_base); > > > > > > And intel_gt_tile_setup then calls __intel_gt_init_early which assigns > > > gt->i915 yet again. > > > > > > So I'd say there is probably space to bring this all into a more > > > streamlined flow, even more than what you suggest below. > > > > yes, I noticed them! > > > > Patch 3, 5 and 10 are very much connected with each other: 3 > > prepares for one tile, 5 prepares for multitile and 10 does the > > multitile. While in between other patches are doing other things. > > > > On top of some cleanups we could also rearrange the patches with > > some squashing and reordering to have them a bit more linear and > > also easier to review. > > Yes. Maybe make intel_gt_tile_setup accept more arguments so it can be truly > used to setup a gt? > > intel_gt_tile_setup(gt, id, name, type, engine_mask) > > The usual thing where patch which adds something extends the prototype to > include more stuff. If that applies here. > > I know it is originally my patch but I don't have the time to rework it, > much less the whole series, so usual dispensation to take over authorship if > changes are large applies. as no one is stepping forward, if you and Matt are OK, I can try to venture in some refactoring of these three patches (3, 5 and 10). Andi
[Bug 214859] drm-amdgpu-init-iommu~fd-device-init.patch introduce bug
https://bugzilla.kernel.org/show_bug.cgi?id=214859 Alex Deucher (alexdeuc...@gmail.com) changed: What|Removed |Added CC||alexdeuc...@gmail.com --- Comment #4 from Alex Deucher (alexdeuc...@gmail.com) --- I think this patch set should address the issue: https://patchwork.freedesktop.org/series/96508/ -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 214901] amdgpu freezes HP laptop at start up
https://bugzilla.kernel.org/show_bug.cgi?id=214901 spassw...@web.de changed: What|Removed |Added Regression|No |Yes -- 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 2/2] drm/i915/ttm: Failsafe migration blits
On Tue, 2 Nov 2021 at 17:55, Thomas Hellström
wrote:
>
>
> On 11/2/21 18:40, Matthew Auld wrote:
> > On Tue, 2 Nov 2021 at 16:39, Thomas Hellström
> > wrote:
> >> If the initial fill blit or copy blit of an object fails, the old
> >> content of the data might be exposed and read as soon as either CPU- or
> >> GPU PTEs are set up to point at the pages.
> >>
> >> Intercept the blit fence with an async callback that checks the
> >> blit fence for errors and if there are errors performs an async cpu blit
> >> instead. If there is a failure to allocate the async dma_fence_work,
> >> allocate it on the stack and sync wait for the blit to complete.
> >>
> >> Add selftests that simulate gpu blit failures and failure to allocate
> >> the async dma_fence_work.
> >>
> >> A previous version of this pach used dma_fence_work, now that's
> >> opencoded which adds more code but might lower the latency
> >> somewhat in the common non-error case.
> >>
> >> v3:
> >> - Style fixes (Matthew Auld)
> >>
> >> Signed-off-by: Thomas Hellström
> >> Reviewed-by: Matthew Auld
> >> ---
> >> drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 322 +++---
> >> drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h | 5 +
> >> .../drm/i915/gem/selftests/i915_gem_migrate.c | 24 +-
> >> 3 files changed, 295 insertions(+), 56 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> >> b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> >> index 0ed6b7f2b95f..b89672c547f8 100644
> >> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> >> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> >> @@ -18,6 +18,29 @@
> >> #include "gt/intel_gt.h"
> >> #include "gt/intel_migrate.h"
> >>
> >> +/**
> >> + * DOC: Selftest failure modes for failsafe migration:
> >> + *
> >> + * For fail_gpu_migration, the gpu blit scheduled is always a clear blit
> >> + * rather than a copy blit, and then we force the failure paths as if
> >> + * the blit fence returned an error.
> >> + *
> >> + * For fail_work_allocation we fail the kmalloc of the async worker, we
> >> + * sync the gpu blit. If it then fails, or fail_gpu_migration is set to
> >> + * true, then a memcpy operation is performed sync.
> >> + */
> >> +#ifdef CONFIG_DRM_I915_SELFTEST
> > When pushing maybe make this:
> >
> > #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> >
> > Which seems to be consistent with most of the other places.
>
> Hmm,
>
> I noticed that i915 is doing that, although I thought these macros were
> primarily intended for C expressions?
i915 is not the only one. AFAIK it just conveniently handles both
bultin and module.
>
> /Thomas
>
>
> >
> >> +static bool fail_gpu_migration;
> >> +static bool fail_work_allocation;
> >> +
> >> +void i915_ttm_migrate_set_failure_modes(bool gpu_migration,
> >> + bool work_allocation)
> >> +{
> >> + fail_gpu_migration = gpu_migration;
> >> + fail_work_allocation = work_allocation;
> >> +}
> >> +#endif
> >> +
Re: [PATCH] drm/amd/display: remove unnecessary conditional operators
Applied. Thanks! Alex On Tue, Nov 2, 2021 at 4:58 AM Simon Ser wrote: > > Reviewed-by: Simon Ser
[Bug 204987] fault in amdgpu_dm_atomic_commit_tail on Vega64 with compton and redshift
https://bugzilla.kernel.org/show_bug.cgi?id=204987 Frank Steinborn (stei...@nognu.de) changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |OBSOLETE -- 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 2/2] drm/i915/ttm: Failsafe migration blits
On 11/2/21 18:40, Matthew Auld wrote:
On Tue, 2 Nov 2021 at 16:39, Thomas Hellström
wrote:
If the initial fill blit or copy blit of an object fails, the old
content of the data might be exposed and read as soon as either CPU- or
GPU PTEs are set up to point at the pages.
Intercept the blit fence with an async callback that checks the
blit fence for errors and if there are errors performs an async cpu blit
instead. If there is a failure to allocate the async dma_fence_work,
allocate it on the stack and sync wait for the blit to complete.
Add selftests that simulate gpu blit failures and failure to allocate
the async dma_fence_work.
A previous version of this pach used dma_fence_work, now that's
opencoded which adds more code but might lower the latency
somewhat in the common non-error case.
v3:
- Style fixes (Matthew Auld)
Signed-off-by: Thomas Hellström
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 322 +++---
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h | 5 +
.../drm/i915/gem/selftests/i915_gem_migrate.c | 24 +-
3 files changed, 295 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
index 0ed6b7f2b95f..b89672c547f8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -18,6 +18,29 @@
#include "gt/intel_gt.h"
#include "gt/intel_migrate.h"
+/**
+ * DOC: Selftest failure modes for failsafe migration:
+ *
+ * For fail_gpu_migration, the gpu blit scheduled is always a clear blit
+ * rather than a copy blit, and then we force the failure paths as if
+ * the blit fence returned an error.
+ *
+ * For fail_work_allocation we fail the kmalloc of the async worker, we
+ * sync the gpu blit. If it then fails, or fail_gpu_migration is set to
+ * true, then a memcpy operation is performed sync.
+ */
+#ifdef CONFIG_DRM_I915_SELFTEST
When pushing maybe make this:
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
Which seems to be consistent with most of the other places.
Hmm,
I noticed that i915 is doing that, although I thought these macros were
primarily intended for C expressions?
/Thomas
+static bool fail_gpu_migration;
+static bool fail_work_allocation;
+
+void i915_ttm_migrate_set_failure_modes(bool gpu_migration,
+ bool work_allocation)
+{
+ fail_gpu_migration = gpu_migration;
+ fail_work_allocation = work_allocation;
+}
+#endif
+
Re: [PATCH 01/13] drm/connector: Add define for HDMI 1.4 Maximum Pixel Rate
On Tue, Nov 2, 2021 at 10:59 AM Maxime Ripard wrote:
>
> A lot of drivers open-code the HDMI 1.4 maximum pixel rate in their
> driver to test whether the resolutions are supported or if the
> scrambling needs to be enabled.
>
> Let's create a common define for everyone to use it.
>
> Cc: Alex Deucher
> Cc: amd-...@lists.freedesktop.org
> Cc: Andrzej Hajda
> Cc: Benjamin Gaignard
> Cc: "Christian König"
> Cc: Emma Anholt
> Cc: intel-...@lists.freedesktop.org
> Cc: Jani Nikula
> Cc: Jernej Skrabec
> Cc: Jerome Brunet
> Cc: Jonas Karlman
> Cc: Jonathan Hunter
> Cc: Joonas Lahtinen
> Cc: Kevin Hilman
> Cc: Laurent Pinchart
> Cc: linux-amlo...@lists.infradead.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-te...@vger.kernel.org
> Cc: Martin Blumenstingl
> Cc: Neil Armstrong
> Cc: "Pan, Xinhui"
> Cc: Robert Foss
> Cc: Rodrigo Vivi
> Cc: Thierry Reding
> Signed-off-by: Maxime Ripard
> ---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 ++--
> drivers/gpu/drm/drm_edid.c | 2 +-
> drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
> drivers/gpu/drm/meson/meson_dw_hdmi.c | 4 ++--
> drivers/gpu/drm/radeon/radeon_encoders.c | 2 +-
For radeon:
Acked-by: Alex Deucher
Note that there are several instances of this in amdgpu as well:
drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c:if
(pixel_clock > 34)
drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c:if
(pixel_clock > 34)
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c:if
(mode->clock > 34)
drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c:if
(mode->clock > 34)
Alex
> drivers/gpu/drm/sti/sti_hdmi_tx3g4c28phy.c | 2 +-
> drivers/gpu/drm/tegra/sor.c| 8
> drivers/gpu/drm/vc4/vc4_hdmi.c | 4 ++--
> include/drm/drm_connector.h| 2 ++
> 9 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 62ae63565d3a..3a58db357be0 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -46,7 +46,7 @@
> /* DW-HDMI Controller >= 0x200a are at least compliant with SCDC version 1 */
> #define SCDC_MIN_SOURCE_VERSION0x1
>
> -#define HDMI14_MAX_TMDSCLK 34000
> +#define HDMI14_MAX_TMDSCLK (DRM_HDMI_14_MAX_TMDS_CLK_KHZ * 1000)
>
> enum hdmi_datamap {
> RGB444_8B = 0x01,
> @@ -1264,7 +1264,7 @@ static bool dw_hdmi_support_scdc(struct dw_hdmi *hdmi,
> * for low rates is not supported either
> */
> if (!display->hdmi.scdc.scrambling.low_rates &&
> - display->max_tmds_clock <= 34)
> + display->max_tmds_clock <= DRM_HDMI_14_MAX_TMDS_CLK_KHZ)
> return false;
>
> return true;
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 7aa2a56a71c8..ec8fb2d098ae 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4966,7 +4966,7 @@ static void drm_parse_hdmi_forum_vsdb(struct
> drm_connector *connector,
> u32 max_tmds_clock = hf_vsdb[5] * 5000;
> struct drm_scdc *scdc = >scdc;
>
> - if (max_tmds_clock > 34) {
> + if (max_tmds_clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ) {
> display->max_tmds_clock = max_tmds_clock;
> DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n",
> display->max_tmds_clock);
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
> b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index d2e61f6c6e08..0666203d52b7 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -2226,7 +2226,7 @@ int intel_hdmi_compute_config(struct intel_encoder
> *encoder,
> if (scdc->scrambling.low_rates)
> pipe_config->hdmi_scrambling = true;
>
> - if (pipe_config->port_clock > 34) {
> + if (pipe_config->port_clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ) {
> pipe_config->hdmi_scrambling = true;
> pipe_config->hdmi_high_tmds_clock_ratio = true;
> }
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index 0afbd1e70bfc..8078667aea0e 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -434,7 +434,7 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void
> *data,
> readl_relaxed(priv->io_base + _REG(VPU_HDMI_SETTING));
>
> DRM_DEBUG_DRIVER("\"%s\" div%d\n", mode->name,
> -mode->clock > 34 ? 40 : 10);
> +mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ ? 40 :
> 10);
>
> /* Enable clocks */
> regmap_update_bits(priv->hhi,
Re: [PATCH v3 2/2] drm/i915/ttm: Failsafe migration blits
On Tue, 2 Nov 2021 at 16:39, Thomas Hellström
wrote:
>
> If the initial fill blit or copy blit of an object fails, the old
> content of the data might be exposed and read as soon as either CPU- or
> GPU PTEs are set up to point at the pages.
>
> Intercept the blit fence with an async callback that checks the
> blit fence for errors and if there are errors performs an async cpu blit
> instead. If there is a failure to allocate the async dma_fence_work,
> allocate it on the stack and sync wait for the blit to complete.
>
> Add selftests that simulate gpu blit failures and failure to allocate
> the async dma_fence_work.
>
> A previous version of this pach used dma_fence_work, now that's
> opencoded which adds more code but might lower the latency
> somewhat in the common non-error case.
>
> v3:
> - Style fixes (Matthew Auld)
>
> Signed-off-by: Thomas Hellström
> Reviewed-by: Matthew Auld
> ---
> drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 322 +++---
> drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h | 5 +
> .../drm/i915/gem/selftests/i915_gem_migrate.c | 24 +-
> 3 files changed, 295 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> index 0ed6b7f2b95f..b89672c547f8 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> @@ -18,6 +18,29 @@
> #include "gt/intel_gt.h"
> #include "gt/intel_migrate.h"
>
> +/**
> + * DOC: Selftest failure modes for failsafe migration:
> + *
> + * For fail_gpu_migration, the gpu blit scheduled is always a clear blit
> + * rather than a copy blit, and then we force the failure paths as if
> + * the blit fence returned an error.
> + *
> + * For fail_work_allocation we fail the kmalloc of the async worker, we
> + * sync the gpu blit. If it then fails, or fail_gpu_migration is set to
> + * true, then a memcpy operation is performed sync.
> + */
> +#ifdef CONFIG_DRM_I915_SELFTEST
When pushing maybe make this:
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
Which seems to be consistent with most of the other places.
> +static bool fail_gpu_migration;
> +static bool fail_work_allocation;
> +
> +void i915_ttm_migrate_set_failure_modes(bool gpu_migration,
> + bool work_allocation)
> +{
> + fail_gpu_migration = gpu_migration;
> + fail_work_allocation = work_allocation;
> +}
> +#endif
> +
Re: [PATCH v8 01/10] drm/vc4: hdmi: Remove the DDC probing for status detection
On Mon, 25 Oct 2021 at 16:29, Maxime Ripard wrote:
>
> Commit 9d44a8d5 ("drm/vc4: Fall back to using an EDID probe in the
> absence of a GPIO.") added some code to read the EDID through DDC in the
> HDMI driver detect hook since the Pi3 had no HPD GPIO back then.
> However, commit b1b8f45b3130 ("ARM: dts: bcm2837: Add missing GPIOs of
> Expander") changed that a couple of years later.
>
> This causes an issue though since some TV (like the LG 55C8) when it
> comes out of standy will deassert the HPD line, but the EDID will
> remain readable.
>
> It causes an issues nn platforms without an HPD GPIO, like the Pi4,
> where the DDC probing will be our primary mean to detect a display, and
> thus we will never detect the HPD pulse. This was fine before since the
> pulse was small enough that we would never detect it, and we also didn't
> have anything (like the scrambler) that needed to be set up in the
> display.
>
> However, now that we have both, the display during the HPD pulse will
> clear its scrambler status, and since we won't detect the
> disconnect/reconnect cycle we will never enable the scrambler back.
>
> As our main reason for that DDC probing is gone, let's just remove it.
>
> Signed-off-by: Maxime Ripard
A quick conversation with Dom does conclude that the old code was odd
in that DDC read attempt was before we checked the Pi4 HPD. If there
is a need to read the DDC, then it should be after HPD (in whatever
form it exists) has been checked.
No need for that to be reinstated at this point, so this patch is fine
as it stands.
Reviewed-by: Dave Stevenson
> ---
> drivers/gpu/drm/vc4/vc4_hdmi.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 7b0cb08e6563..338968275724 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -193,8 +193,6 @@ vc4_hdmi_connector_detect(struct drm_connector
> *connector, bool force)
> if (vc4_hdmi->hpd_gpio &&
> gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio)) {
> connected = true;
> - } else if (drm_probe_ddc(vc4_hdmi->ddc)) {
> - connected = true;
> } else {
> unsigned long flags;
> u32 hotplug;
> --
> 2.31.1
>
Re: [PATCH v8 10/10] drm/vc4: Increase the core clock based on HVS load
On Mon, 25 Oct 2021 at 16:29, Maxime Ripard wrote:
>
> Depending on a given HVS output (HVS to PixelValves) and input (planes
> attached to a channel) load, the HVS needs for the core clock to be
> raised above its boot time default.
>
> Failing to do so will result in a vblank timeout and a stalled display
> pipeline.
>
> Signed-off-by: Maxime Ripard
I will make the comment that this does a load of computation of hvs
load when running on hvs4 (BCM2835/6/7), even though it's redundant on
those chips.
The overhead is relatively minimal, but could be bypassed if viewed necessary.
Otherwise
Reviewed-by: Dave Stevenson
> ---
> drivers/gpu/drm/vc4/vc4_crtc.c | 15 +
> drivers/gpu/drm/vc4/vc4_drv.h | 2 +
> drivers/gpu/drm/vc4/vc4_kms.c | 110 ++---
> 3 files changed, 118 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index 6decaa12a078..287dbc89ad64 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -659,12 +659,27 @@ static int vc4_crtc_atomic_check(struct drm_crtc *crtc,
> struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc_state);
> struct drm_connector *conn;
> struct drm_connector_state *conn_state;
> + struct drm_encoder *encoder;
> int ret, i;
>
> ret = vc4_hvs_atomic_check(crtc, state);
> if (ret)
> return ret;
>
> + encoder = vc4_get_crtc_encoder(crtc, crtc_state);
> + if (encoder) {
> + const struct drm_display_mode *mode =
> _state->adjusted_mode;
> + struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
> +
> + mode = _state->adjusted_mode;
> + if (vc4_encoder->type == VC4_ENCODER_TYPE_HDMI0) {
> + vc4_state->hvs_load = max(mode->clock *
> mode->hdisplay / mode->htotal + 1000,
> + mode->clock * 9 / 10) *
> 1000;
> + } else {
> + vc4_state->hvs_load = mode->clock * 1000;
> + }
> + }
> +
> for_each_new_connector_in_state(state, conn, conn_state,
> i) {
> if (conn_state->crtc != crtc)
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 813c5d0ea98e..4329e09d357c 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -558,6 +558,8 @@ struct vc4_crtc_state {
> unsigned int bottom;
> } margins;
>
> + unsigned long hvs_load;
> +
> /* Transitional state below, only valid during atomic commits */
> bool update_muxing;
> };
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index 41cb4869da50..79d4d9dd1394 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -39,9 +39,11 @@ static struct vc4_ctm_state *to_vc4_ctm_state(struct
> drm_private_state *priv)
>
> struct vc4_hvs_state {
> struct drm_private_state base;
> + unsigned long core_clock_rate;
>
> struct {
> unsigned in_use: 1;
> + unsigned long fifo_load;
> struct drm_crtc_commit *pending_commit;
> } fifo_state[HVS_NUM_CHANNELS];
> };
> @@ -340,10 +342,19 @@ static void vc4_atomic_commit_tail(struct
> drm_atomic_state *state)
> struct vc4_hvs *hvs = vc4->hvs;
> struct drm_crtc_state *old_crtc_state;
> struct drm_crtc_state *new_crtc_state;
> + struct vc4_hvs_state *new_hvs_state;
> struct drm_crtc *crtc;
> struct vc4_hvs_state *old_hvs_state;
> int i;
>
> + old_hvs_state = vc4_hvs_get_old_global_state(state);
> + if (WARN_ON(!old_hvs_state))
> + return;
> +
> + new_hvs_state = vc4_hvs_get_new_global_state(state);
> + if (WARN_ON(!new_hvs_state))
> + return;
> +
> for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> struct vc4_crtc_state *vc4_crtc_state;
>
> @@ -354,12 +365,13 @@ static void vc4_atomic_commit_tail(struct
> drm_atomic_state *state)
> vc4_hvs_mask_underrun(dev, vc4_crtc_state->assigned_channel);
> }
>
> - if (vc4->hvs->hvs5)
> - clk_set_min_rate(hvs->core_clk, 5);
> + if (vc4->hvs->hvs5) {
> + unsigned long core_rate = max_t(unsigned long,
> + 5,
> +
> new_hvs_state->core_clock_rate);
>
> - old_hvs_state = vc4_hvs_get_old_global_state(state);
> - if (!old_hvs_state)
> - return;
> + clk_set_min_rate(hvs->core_clk, core_rate);
> + }
>
> for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {
> struct vc4_crtc_state
Re: [PATCH v8 09/10] drm/vc4: hdmi: Enable the scrambler on reconnection
On Mon, 25 Oct 2021 at 16:29, Maxime Ripard wrote:
>
> If we have a state already and disconnect/reconnect the display, the
> SCDC messages won't be sent again since we didn't go through a disable /
> enable cycle.
>
> In order to fix this, let's call the vc4_hdmi_enable_scrambling function
> in the detect callback if there is a mode and it needs the scrambler to
> be enabled.
>
> Fixes: c85695a2016e ("drm/vc4: hdmi: Enable the scrambler")
> Signed-off-by: Maxime Ripard
Reviewed-by: Dave Stevenson
> ---
> drivers/gpu/drm/vc4/vc4_hdmi.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index d36b3b6ebed1..fab9b93e1b84 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -180,6 +180,8 @@ static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi
> *vc4_hdmi)
> static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi) {}
> #endif
>
> +static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder);
> +
> static enum drm_connector_status
> vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
> {
> @@ -216,6 +218,7 @@ vc4_hdmi_connector_detect(struct drm_connector
> *connector, bool force)
> }
> }
>
> + vc4_hdmi_enable_scrambling(_hdmi->encoder.base.base);
> pm_runtime_put(_hdmi->pdev->dev);
> mutex_unlock(_hdmi->mutex);
> return connector_status_connected;
> --
> 2.31.1
>
Re: [PATCH v8 07/10] drm/vc4: Leverage the load tracker on the BCM2711
On Mon, 25 Oct 2021 at 16:29, Maxime Ripard wrote:
>
> The load tracker was initially designed to report and warn about a load
> too high for the HVS. To do so, it computes for each plane the impact
> it's going to have on the HVS, and will warn (if it's enabled) if we go
> over what the hardware can process.
>
> While the limits being used are a bit irrelevant to the BCM2711, the
> algorithm to compute the HVS load will be one component used in order to
> compute the core clock rate on the BCM2711.
>
> Let's remove the hooks to prevent the load tracker to do its
> computation, but since we don't have the same limits, don't check them
> against them, and prevent the debugfs file to enable it from being
> created.
>
> Signed-off-by: Maxime Ripard
Reviewed-by: Dave Stevenson
> ---
> drivers/gpu/drm/vc4/vc4_debugfs.c | 7 +--
> drivers/gpu/drm/vc4/vc4_drv.h | 3 ---
> drivers/gpu/drm/vc4/vc4_kms.c | 16 +---
> drivers/gpu/drm/vc4/vc4_plane.c | 5 -
> 4 files changed, 10 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_debugfs.c
> b/drivers/gpu/drm/vc4/vc4_debugfs.c
> index 6da22af4ee91..ba2d8ea562af 100644
> --- a/drivers/gpu/drm/vc4/vc4_debugfs.c
> +++ b/drivers/gpu/drm/vc4/vc4_debugfs.c
> @@ -7,6 +7,7 @@
> #include
> #include
> #include
> +#include
>
> #include "vc4_drv.h"
> #include "vc4_regs.h"
> @@ -26,8 +27,10 @@ vc4_debugfs_init(struct drm_minor *minor)
> struct vc4_dev *vc4 = to_vc4_dev(minor->dev);
> struct vc4_debugfs_info_entry *entry;
>
> - debugfs_create_bool("hvs_load_tracker", S_IRUGO | S_IWUSR,
> - minor->debugfs_root, >load_tracker_enabled);
> + if (!of_device_is_compatible(vc4->hvs->pdev->dev.of_node,
> +"brcm,bcm2711-vc5"))
> + debugfs_create_bool("hvs_load_tracker", S_IRUGO | S_IWUSR,
> + minor->debugfs_root,
> >load_tracker_enabled);
>
> list_for_each_entry(entry, >debugfs_list, link) {
> drm_debugfs_create_files(>info, 1,
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 60826aca9e5b..813c5d0ea98e 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -202,9 +202,6 @@ struct vc4_dev {
>
> int power_refcount;
>
> - /* Set to true when the load tracker is supported. */
> - bool load_tracker_available;
> -
> /* Set to true when the load tracker is active. */
> bool load_tracker_enabled;
>
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index 028f19f7a5f8..41cb4869da50 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -552,9 +552,6 @@ static int vc4_load_tracker_atomic_check(struct
> drm_atomic_state *state)
> struct drm_plane *plane;
> int i;
>
> - if (!vc4->load_tracker_available)
> - return 0;
> -
> priv_state = drm_atomic_get_private_obj_state(state,
> >load_tracker);
> if (IS_ERR(priv_state))
> @@ -629,9 +626,6 @@ static void vc4_load_tracker_obj_fini(struct drm_device
> *dev, void *unused)
> {
> struct vc4_dev *vc4 = to_vc4_dev(dev);
>
> - if (!vc4->load_tracker_available)
> - return;
> -
> drm_atomic_private_obj_fini(>load_tracker);
> }
>
> @@ -639,9 +633,6 @@ static int vc4_load_tracker_obj_init(struct vc4_dev *vc4)
> {
> struct vc4_load_tracker_state *load_state;
>
> - if (!vc4->load_tracker_available)
> - return 0;
> -
> load_state = kzalloc(sizeof(*load_state), GFP_KERNEL);
> if (!load_state)
> return -ENOMEM;
> @@ -869,9 +860,12 @@ int vc4_kms_load(struct drm_device *dev)
> "brcm,bcm2711-vc5");
> int ret;
>
> + /*
> +* The limits enforced by the load tracker aren't relevant for
> +* the BCM2711, but the load tracker computations are used for
> +* the core clock rate calculation.
> +*/
> if (!is_vc5) {
> - vc4->load_tracker_available = true;
> -
> /* Start with the load tracker enabled. Can be
> * disabled through the debugfs load_tracker file.
> */
> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
> index 19161b6ab27f..ac761c683663 100644
> --- a/drivers/gpu/drm/vc4/vc4_plane.c
> +++ b/drivers/gpu/drm/vc4/vc4_plane.c
> @@ -529,11 +529,6 @@ static void vc4_plane_calc_load(struct drm_plane_state
> *state)
> struct vc4_plane_state *vc4_state;
> struct drm_crtc_state *crtc_state;
> unsigned int vscale_factor;
> - struct vc4_dev *vc4;
> -
> - vc4 = to_vc4_dev(state->plane->dev);
> - if (!vc4->load_tracker_available)
> - return;
>
>
Re: [PATCH 29/29] drm/i915/gvt: merge gvt.c into kvmgvt.c
On Tue, Nov 02, 2021 at 08:06:01AM +0100, Christoph Hellwig wrote: > The code in both files is deeply interconnected, so merge it and > keep a bunch of structures and functions static. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/Makefile| 1 - > drivers/gpu/drm/i915/gvt/gvt.c | 291 --- > drivers/gpu/drm/i915/gvt/gvt.h | 6 - > drivers/gpu/drm/i915/gvt/kvmgt.c | 264 +++- > 4 files changed, 260 insertions(+), 302 deletions(-) > delete mode 100644 drivers/gpu/drm/i915/gvt/gvt.c Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 28/29] drm/i915/gvt: convert to use vfio_register_group_dev()
On Tue, Nov 02, 2021 at 08:06:00AM +0100, Christoph Hellwig wrote: > This is straightforward conversion, the intel_vgpu already has a pointer > to the vfio_dev, which can be replaced with the embedded structure and > we can replace all the mdev_get_drvdata() with a simple container_of(). This should be using vfio_register_emulated_iommu_dev(), right? I expect drivers using the pin API to use the emulated_iommu_dev() interface at least.. Otherwise this looks good Reviewed-by: Jason Gunthorpe Jason
[PATCH v3 2/2] drm/i915/ttm: Failsafe migration blits
If the initial fill blit or copy blit of an object fails, the old
content of the data might be exposed and read as soon as either CPU- or
GPU PTEs are set up to point at the pages.
Intercept the blit fence with an async callback that checks the
blit fence for errors and if there are errors performs an async cpu blit
instead. If there is a failure to allocate the async dma_fence_work,
allocate it on the stack and sync wait for the blit to complete.
Add selftests that simulate gpu blit failures and failure to allocate
the async dma_fence_work.
A previous version of this pach used dma_fence_work, now that's
opencoded which adds more code but might lower the latency
somewhat in the common non-error case.
v3:
- Style fixes (Matthew Auld)
Signed-off-by: Thomas Hellström
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 322 +++---
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h | 5 +
.../drm/i915/gem/selftests/i915_gem_migrate.c | 24 +-
3 files changed, 295 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
index 0ed6b7f2b95f..b89672c547f8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -18,6 +18,29 @@
#include "gt/intel_gt.h"
#include "gt/intel_migrate.h"
+/**
+ * DOC: Selftest failure modes for failsafe migration:
+ *
+ * For fail_gpu_migration, the gpu blit scheduled is always a clear blit
+ * rather than a copy blit, and then we force the failure paths as if
+ * the blit fence returned an error.
+ *
+ * For fail_work_allocation we fail the kmalloc of the async worker, we
+ * sync the gpu blit. If it then fails, or fail_gpu_migration is set to
+ * true, then a memcpy operation is performed sync.
+ */
+#ifdef CONFIG_DRM_I915_SELFTEST
+static bool fail_gpu_migration;
+static bool fail_work_allocation;
+
+void i915_ttm_migrate_set_failure_modes(bool gpu_migration,
+ bool work_allocation)
+{
+ fail_gpu_migration = gpu_migration;
+ fail_work_allocation = work_allocation;
+}
+#endif
+
static enum i915_cache_level
i915_ttm_cache_level(struct drm_i915_private *i915, struct ttm_resource *res,
struct ttm_tt *ttm)
@@ -129,11 +152,11 @@ int i915_ttm_move_notify(struct ttm_buffer_object *bo)
return 0;
}
-static int i915_ttm_accel_move(struct ttm_buffer_object *bo,
- bool clear,
- struct ttm_resource *dst_mem,
- struct ttm_tt *dst_ttm,
- struct sg_table *dst_st)
+static struct dma_fence *i915_ttm_accel_move(struct ttm_buffer_object *bo,
+bool clear,
+struct ttm_resource *dst_mem,
+struct ttm_tt *dst_ttm,
+struct sg_table *dst_st)
{
struct drm_i915_private *i915 = container_of(bo->bdev, typeof(*i915),
bdev);
@@ -144,30 +167,29 @@ static int i915_ttm_accel_move(struct ttm_buffer_object
*bo,
int ret;
if (!i915->gt.migrate.context || intel_gt_is_wedged(>gt))
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
+
+ /* With fail_gpu_migration, we always perform a GPU clear. */
+ if (I915_SELFTEST_ONLY(fail_gpu_migration))
+ clear = true;
dst_level = i915_ttm_cache_level(i915, dst_mem, dst_ttm);
if (clear) {
- if (bo->type == ttm_bo_type_kernel)
- return -EINVAL;
+ if (bo->type == ttm_bo_type_kernel &&
+ !I915_SELFTEST_ONLY(fail_gpu_migration))
+ return ERR_PTR(-EINVAL);
intel_engine_pm_get(i915->gt.migrate.context->engine);
ret = intel_context_migrate_clear(i915->gt.migrate.context,
NULL,
dst_st->sgl, dst_level,
i915_ttm_gtt_binds_lmem(dst_mem),
0, );
-
- if (!ret && rq) {
- i915_request_wait(rq, 0, MAX_SCHEDULE_TIMEOUT);
- i915_request_put(rq);
- }
- intel_engine_pm_put(i915->gt.migrate.context->engine);
} else {
struct i915_refct_sgt *src_rsgt =
i915_ttm_resource_get_st(obj, bo->resource);
if (IS_ERR(src_rsgt))
- return PTR_ERR(src_rsgt);
+ return ERR_CAST(src_rsgt);
src_level = i915_ttm_cache_level(i915, bo->resource, src_ttm);
intel_engine_pm_get(i915->gt.migrate.context->engine);
@@ -178,15 +200,182 @@ static int
[PATCH v3 0/2] drm/i915: Failsafe migration blits
This patch series introduces failsafe migration blits. The reason for this seemingly strange concept is that if the initial clearing or readback of LMEM fails for some reason[1], and we then set up either GPU- or CPU ptes to the allocated LMEM, we can expose old contents from other clients. So after each migration blit to LMEM, attach a dma-fence callback that checks the migration fence error value and if it's an error, performs a memcpy blit, instead. Patch 1 splits out the TTM move code into separate files Patch 2 implements the failsafe blits and related self-tests [1] There are at least two ways we could trigger exposure of uninitialized LMEM assuming the migration blits themselves never trigger a gpu hang. a) A gpu operation preceding a pipelined eviction blit resets and sets the error fence to -EIO, and the error is propagated across the TTM manager to the clear / swapin blit of a newly allocated TTM resource. It aborts and leaves the memory uninitialized. b) Something wedges the GT while a migration blit is submitted. It ends up never executed and TTM can fault user-space cpu-ptes into uninitialized memory. v3: - Style fixes in second patch (Matthew Auld) Thomas Hellström (2): drm/i915/ttm: Reorganize the ttm move code drm/i915/ttm: Failsafe migration blits drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 328 ++- drivers/gpu/drm/i915/gem/i915_gem_ttm.h | 35 ++ drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 520 ++ drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h | 43 ++ .../drm/i915/gem/selftests/i915_gem_migrate.c | 24 +- 6 files changed, 670 insertions(+), 281 deletions(-) create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h -- 2.31.1
[PATCH v3 1/2] drm/i915/ttm: Reorganize the ttm move code
We are about to introduce failsafe- and asynchronous migration and
ttm moves.
This will add complexity and code to the TTM move code so it makes sense
to split it out to a separate file to make the i915 TTM code easer to
digest.
Split the i915 TTM move code out and since we will have to change the name
of the gpu_binds_iomem() and cpu_maps_iomem() functions anyway,
we alter the name of gpu_binds_iomem() to i915_ttm_gtt_binds_lmem() which
is more reflecting what it is used for.
With this we also add some more documentation. Otherwise there should be
no functional change.
Signed-off-by: Thomas Hellström
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/Makefile| 1 +
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 328 +++
drivers/gpu/drm/i915/gem/i915_gem_ttm.h | 35 ++
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 308 +
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h | 38 +++
5 files changed, 430 insertions(+), 280 deletions(-)
create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 467872cca027..7d0d0b814670 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -154,6 +154,7 @@ gem-y += \
gem/i915_gem_throttle.o \
gem/i915_gem_tiling.o \
gem/i915_gem_ttm.o \
+ gem/i915_gem_ttm_move.o \
gem/i915_gem_ttm_pm.o \
gem/i915_gem_userptr.o \
gem/i915_gem_wait.o \
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 6a05369e2705..6369fb9b2455 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -14,13 +14,9 @@
#include "gem/i915_gem_object.h"
#include "gem/i915_gem_region.h"
#include "gem/i915_gem_ttm.h"
+#include "gem/i915_gem_ttm_move.h"
#include "gem/i915_gem_ttm_pm.h"
-
-#include "gt/intel_engine_pm.h"
-#include "gt/intel_gt.h"
-#include "gt/intel_migrate.h"
-
#define I915_TTM_PRIO_PURGE 0
#define I915_TTM_PRIO_NO_PAGES 1
#define I915_TTM_PRIO_HAS_PAGES 2
@@ -108,28 +104,6 @@ static int i915_ttm_err_to_gem(int err)
return err;
}
-static bool gpu_binds_iomem(struct ttm_resource *mem)
-{
- return mem->mem_type != TTM_PL_SYSTEM;
-}
-
-static bool cpu_maps_iomem(struct ttm_resource *mem)
-{
- /* Once / if we support GGTT, this is also false for cached ttm_tts */
- return mem->mem_type != TTM_PL_SYSTEM;
-}
-
-static enum i915_cache_level
-i915_ttm_cache_level(struct drm_i915_private *i915, struct ttm_resource *res,
-struct ttm_tt *ttm)
-{
- return ((HAS_LLC(i915) || HAS_SNOOP(i915)) && !gpu_binds_iomem(res) &&
- ttm->caching == ttm_cached) ? I915_CACHE_LLC :
- I915_CACHE_NONE;
-}
-
-static void i915_ttm_adjust_lru(struct drm_i915_gem_object *obj);
-
static enum ttm_caching
i915_ttm_select_tt_caching(const struct drm_i915_gem_object *obj)
{
@@ -370,23 +344,14 @@ static void i915_ttm_evict_flags(struct ttm_buffer_object
*bo,
*placement = i915_sys_placement;
}
-static int i915_ttm_move_notify(struct ttm_buffer_object *bo)
-{
- struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
- int ret;
-
- ret = i915_gem_object_unbind(obj, I915_GEM_OBJECT_UNBIND_ACTIVE);
- if (ret)
- return ret;
-
- ret = __i915_gem_object_put_pages(obj);
- if (ret)
- return ret;
-
- return 0;
-}
-
-static void i915_ttm_free_cached_io_rsgt(struct drm_i915_gem_object *obj)
+/**
+ * i915_ttm_free_cached_io_rsgt - Free object cached LMEM information
+ * @obj: The GEM object
+ * This function frees any LMEM-related information that is cached on
+ * the object. For example the radix tree for fast page lookup and the
+ * cached refcounted sg-table
+ */
+void i915_ttm_free_cached_io_rsgt(struct drm_i915_gem_object *obj)
{
struct radix_tree_iter iter;
void __rcu **slot;
@@ -403,56 +368,16 @@ static void i915_ttm_free_cached_io_rsgt(struct
drm_i915_gem_object *obj)
obj->ttm.cached_io_rsgt = NULL;
}
-static void
-i915_ttm_adjust_domains_after_move(struct drm_i915_gem_object *obj)
-{
- struct ttm_buffer_object *bo = i915_gem_to_ttm(obj);
-
- if (cpu_maps_iomem(bo->resource) || bo->ttm->caching != ttm_cached) {
- obj->write_domain = I915_GEM_DOMAIN_WC;
- obj->read_domains = I915_GEM_DOMAIN_WC;
- } else {
- obj->write_domain = I915_GEM_DOMAIN_CPU;
- obj->read_domains = I915_GEM_DOMAIN_CPU;
- }
-}
-
-static void i915_ttm_adjust_gem_after_move(struct drm_i915_gem_object *obj)
-{
- struct ttm_buffer_object *bo = i915_gem_to_ttm(obj);
- unsigned int cache_level;
- unsigned int i;
-
- /*
-* If object was moved to an allowable region, update the object
-
Re: [PATCH 27/29] drm/i915/gvt: remove kvmgt_guest_{init,exit}
On Tue, Nov 02, 2021 at 08:05:59AM +0100, Christoph Hellwig wrote: > Merge these into their only callers. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/kvmgt.c | 129 ++- > 1 file changed, 60 insertions(+), 69 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH v8 06/10] drm/vc4: crtc: Add some logging
On Mon, 25 Oct 2021 at 16:29, Maxime Ripard wrote: > > The encoder retrieval code has been a source of bugs and glitches in the > past and the crtc <-> encoder association been wrong in a number of > different ways. > > Add some logging to quickly spot issues if they occur. > > Signed-off-by: Maxime Ripard Reviewed-by: Dave Stevenson > --- > drivers/gpu/drm/vc4/vc4_crtc.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c > index fbc1d4638650..6decaa12a078 100644 > --- a/drivers/gpu/drm/vc4/vc4_crtc.c > +++ b/drivers/gpu/drm/vc4/vc4_crtc.c > @@ -541,6 +541,9 @@ static void vc4_crtc_atomic_disable(struct drm_crtc *crtc, > struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, old_state); > struct drm_device *dev = crtc->dev; > > + drm_dbg(dev, "Disabling CRTC %s (%u) connected to Encoder %s (%u)", > + crtc->name, crtc->base.id, encoder->name, encoder->base.id); > + > require_hvs_enabled(dev); > > /* Disable vblank irq handling before crtc is disabled. */ > @@ -572,6 +575,9 @@ static void vc4_crtc_atomic_enable(struct drm_crtc *crtc, > struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, new_state); > struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder); > > + drm_dbg(dev, "Enabling CRTC %s (%u) connected to Encoder %s (%u)", > + crtc->name, crtc->base.id, encoder->name, encoder->base.id); > + > require_hvs_enabled(dev); > > /* Enable vblank irq handling before crtc is started otherwise > -- > 2.31.1 >
Re: [PATCH v8 05/10] drm/vc4: crtc: Rework the encoder retrieval code (again)
On Mon, 25 Oct 2021 at 16:29, Maxime Ripard wrote:
>
> It turns out the encoder retrieval code, in addition to being
> unnecessarily complicated, has a bug when only the planes and crtcs are
> affected by a given atomic commit.
>
> Indeed, in such a case, either drm_atomic_get_old_connector_state or
> drm_atomic_get_new_connector_state will return NULL and thus our encoder
> retrieval code will not match on anything.
>
> We can however simplify the code by using drm_for_each_encoder_mask, the
> drm_crtc_state storing the encoders a given CRTC is connected to
> directly and without relying on any other state.
>
> Signed-off-by: Maxime Ripard
Reviewed-by: Dave Stevenson
> ---
> drivers/gpu/drm/vc4/vc4_crtc.c | 30 +-
> drivers/gpu/drm/vc4/vc4_drv.h | 4 +---
> 2 files changed, 10 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index e5c2e29a6f01..fbc1d4638650 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -282,26 +282,14 @@ static u32 vc4_crtc_get_fifo_full_level_bits(struct
> vc4_crtc *vc4_crtc,
> * same CRTC.
> */
> struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc,
> -struct drm_atomic_state *state,
> -struct drm_connector_state
> *(*get_state)(struct drm_atomic_state *state,
> -
> struct drm_connector *connector))
> +struct drm_crtc_state *state)
> {
> - struct drm_connector *connector;
> - struct drm_connector_list_iter conn_iter;
> + struct drm_encoder *encoder;
>
> - drm_connector_list_iter_begin(crtc->dev, _iter);
> - drm_for_each_connector_iter(connector, _iter) {
> - struct drm_connector_state *conn_state = get_state(state,
> connector);
> + WARN_ON(hweight32(state->encoder_mask) > 1);
>
> - if (!conn_state)
> - continue;
> -
> - if (conn_state->crtc == crtc) {
> - drm_connector_list_iter_end(_iter);
> - return connector->encoder;
> - }
> - }
> - drm_connector_list_iter_end(_iter);
> + drm_for_each_encoder_mask(encoder, crtc->dev, state->encoder_mask)
> + return encoder;
>
> return NULL;
> }
> @@ -550,8 +538,7 @@ static void vc4_crtc_atomic_disable(struct drm_crtc *crtc,
> struct drm_crtc_state *old_state =
> drm_atomic_get_old_crtc_state(state,
>
> crtc);
> struct vc4_crtc_state *old_vc4_state = to_vc4_crtc_state(old_state);
> - struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, state,
> -
> drm_atomic_get_old_connector_state);
> + struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, old_state);
> struct drm_device *dev = crtc->dev;
>
> require_hvs_enabled(dev);
> @@ -578,10 +565,11 @@ static void vc4_crtc_atomic_disable(struct drm_crtc
> *crtc,
> static void vc4_crtc_atomic_enable(struct drm_crtc *crtc,
>struct drm_atomic_state *state)
> {
> + struct drm_crtc_state *new_state =
> drm_atomic_get_new_crtc_state(state,
> +
> crtc);
> struct drm_device *dev = crtc->dev;
> struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
> - struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, state,
> -
> drm_atomic_get_new_connector_state);
> + struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, new_state);
> struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
>
> require_hvs_enabled(dev);
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index f5e678491502..60826aca9e5b 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -545,9 +545,7 @@ vc4_crtc_to_vc4_pv_data(const struct vc4_crtc *crtc)
> }
>
> struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc,
> -struct drm_atomic_state *state,
> -struct drm_connector_state
> *(*get_state)(struct drm_atomic_state *state,
> -
> struct drm_connector *connector));
> +struct drm_crtc_state *state);
>
> struct vc4_crtc_state {
> struct drm_crtc_state base;
> --
> 2.31.1
>
Re: [PATCH v8 04/10] drm/vc4: crtc: Add encoder to vc4_crtc_config_pv prototype
On Mon, 25 Oct 2021 at 16:29, Maxime Ripard wrote:
>
> vc4_crtc_config_pv() retrieves the encoder again, even though its only
> caller, vc4_crtc_atomic_enable(), already did.
>
> Pass the encoder pointer as an argument instead of going through all the
> connectors to retrieve it again.
>
> Signed-off-by: Maxime Ripard
Reviewed-by: Dave Stevenson
> ---
> drivers/gpu/drm/vc4/vc4_crtc.c | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index 7cfd4a097847..e5c2e29a6f01 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -315,12 +315,11 @@ static void vc4_crtc_pixelvalve_reset(struct drm_crtc
> *crtc)
> CRTC_WRITE(PV_CONTROL, CRTC_READ(PV_CONTROL) | PV_CONTROL_FIFO_CLR);
> }
>
> -static void vc4_crtc_config_pv(struct drm_crtc *crtc, struct
> drm_atomic_state *state)
> +static void vc4_crtc_config_pv(struct drm_crtc *crtc, struct drm_encoder
> *encoder,
> + struct drm_atomic_state *state)
> {
> struct drm_device *dev = crtc->dev;
> struct vc4_dev *vc4 = to_vc4_dev(dev);
> - struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, state,
> -
> drm_atomic_get_new_connector_state);
> struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
> struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
> const struct vc4_pv_data *pv_data = vc4_crtc_to_vc4_pv_data(vc4_crtc);
> @@ -597,7 +596,7 @@ static void vc4_crtc_atomic_enable(struct drm_crtc *crtc,
> if (vc4_encoder->pre_crtc_configure)
> vc4_encoder->pre_crtc_configure(encoder, state);
>
> - vc4_crtc_config_pv(crtc, state);
> + vc4_crtc_config_pv(crtc, encoder, state);
>
> CRTC_WRITE(PV_CONTROL, CRTC_READ(PV_CONTROL) | PV_CONTROL_EN);
>
> --
> 2.31.1
>
Re: [PATCH v8 03/10] drm/vc4: Make vc4_crtc_get_encoder public
On Mon, 25 Oct 2021 at 16:29, Maxime Ripard wrote:
>
> We'll need that function in vc4_kms to compute the core clock rate
> requirements.
>
> Signed-off-by: Maxime Ripard
Reviewed-by: Dave Stevenson
> ---
> drivers/gpu/drm/vc4/vc4_crtc.c | 8
> drivers/gpu/drm/vc4/vc4_drv.h | 5 +
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index e3ed52d96f42..7cfd4a097847 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -281,10 +281,10 @@ static u32 vc4_crtc_get_fifo_full_level_bits(struct
> vc4_crtc *vc4_crtc,
> * allows drivers to push pixels to more than one encoder from the
> * same CRTC.
> */
> -static struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc,
> - struct drm_atomic_state
> *state,
> - struct drm_connector_state
> *(*get_state)(struct drm_atomic_state *state,
> -
>struct drm_connector *connector))
> +struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc,
> +struct drm_atomic_state *state,
> +struct drm_connector_state
> *(*get_state)(struct drm_atomic_state *state,
> +
> struct drm_connector *connector))
> {
> struct drm_connector *connector;
> struct drm_connector_list_iter conn_iter;
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 4b550ebd9572..f5e678491502 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -544,6 +544,11 @@ vc4_crtc_to_vc4_pv_data(const struct vc4_crtc *crtc)
> return container_of(data, struct vc4_pv_data, base);
> }
>
> +struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc,
> +struct drm_atomic_state *state,
> +struct drm_connector_state
> *(*get_state)(struct drm_atomic_state *state,
> +
> struct drm_connector *connector));
> +
> struct vc4_crtc_state {
> struct drm_crtc_state base;
> /* Dlist area for this CRTC configuration. */
> --
> 2.31.1
>
Re: [PATCH 26/29] drm/i915/gvt: pass a struct intel_vgpu to the vfio read/write helpers
On Tue, Nov 02, 2021 at 08:05:58AM +0100, Christoph Hellwig wrote: > Pass the structure we actually care about instead of deriving it from > the mdev_device in the lower level code. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/kvmgt.c | 28 ++-- > 1 file changed, 14 insertions(+), 14 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 25/29] drm/i915/gvt: streamline intel_vgpu_create
On Tue, Nov 02, 2021 at 08:05:57AM +0100, Christoph Hellwig wrote: > Initialize variables at declaration time, avoid pointless gotos and > cater for the fact that intel_gvt_create_vgpu can't return NULL. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/kvmgt.c | 28 +--- > 1 file changed, 9 insertions(+), 19 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 24/29] drm/i915/gvt: remove the extra vfio_device refcounting for dmabufs
On Tue, Nov 02, 2021 at 08:05:56AM +0100, Christoph Hellwig wrote: > All the dmabufs are torn down when th VGPU is released, so there is > no need for extra refcounting here. > > Based on an patch from Jason Gunthorpe. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/dmabuf.c | 12 > drivers/gpu/drm/i915/gvt/gvt.h| 1 - > 2 files changed, 13 deletions(-) Oh, here it gets fixed up, yes: Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 23/29] drm/i915/gvt: remove struct intel_gvt_mpt
On Tue, Nov 02, 2021 at 08:05:55AM +0100, Christoph Hellwig wrote: > Just call the initializion and exit functions directly and remove > this abstraction entirely. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gvt.c | 11 - > drivers/gpu/drm/i915/gvt/gvt.h | 12 ++--- > drivers/gpu/drm/i915/gvt/hypercall.h | 50 --- > drivers/gpu/drm/i915/gvt/kvmgt.c | 39 ++- > drivers/gpu/drm/i915/gvt/mpt.h | 74 > 5 files changed, 17 insertions(+), 169 deletions(-) > delete mode 100644 drivers/gpu/drm/i915/gvt/hypercall.h > delete mode 100644 drivers/gpu/drm/i915/gvt/mpt.h Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 22/29] drm/i915/gvt: devirtualize dma_pin_guest_page
On Tue, Nov 02, 2021 at 08:05:54AM +0100, Christoph Hellwig wrote: > Just call the function directly and remove a pointless wrapper. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/dmabuf.c| 14 +- > drivers/gpu/drm/i915/gvt/gvt.h | 1 + > drivers/gpu/drm/i915/gvt/hypercall.h | 2 -- > drivers/gpu/drm/i915/gvt/kvmgt.c | 4 +--- > drivers/gpu/drm/i915/gvt/mpt.h | 15 --- > 5 files changed, 3 insertions(+), 33 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 21/29] drm/i915/gvt: devirtualize ->dma_{,un}map_guest_page
On Tue, Nov 02, 2021 at 08:05:53AM +0100, Christoph Hellwig wrote: > Just call the functions directly. Also remove a pointless wrapper. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/dmabuf.c| 10 ++ > drivers/gpu/drm/i915/gvt/gtt.c | 20 +-- > drivers/gpu/drm/i915/gvt/gvt.h | 4 > drivers/gpu/drm/i915/gvt/hypercall.h | 5 - > drivers/gpu/drm/i915/gvt/kvmgt.c | 6 ++ > drivers/gpu/drm/i915/gvt/mpt.h | 29 > 6 files changed, 17 insertions(+), 57 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH v8 02/10] drm/vc4: hdmi: Fix HPD GPIO detection
On Mon, 25 Oct 2021 at 16:29, Maxime Ripard wrote:
>
> Prior to commit 6800234ceee0 ("drm/vc4: hdmi: Convert to gpiod"), in the
> detect hook, if we had an HPD GPIO we would only rely on it and return
> whatever state it was in.
>
> However, that commit changed that by mistake to only consider the case
> where we have a GPIO and it returns a logical high, and would fall back
> to the other methods otherwise.
>
> Since we can read the EDIDs when the HPD signal is low on some displays,
> we changed the detection status from disconnected to connected, and we
> would ignore an HPD pulse.
>
> Fixes: 6800234ceee0 ("drm/vc4: hdmi: Convert to gpiod")
> Signed-off-by: Maxime Ripard
Reviewed-by: Dave Stevenson
> ---
> drivers/gpu/drm/vc4/vc4_hdmi.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 338968275724..dde67b991ae7 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -190,9 +190,9 @@ vc4_hdmi_connector_detect(struct drm_connector
> *connector, bool force)
>
> WARN_ON(pm_runtime_resume_and_get(_hdmi->pdev->dev));
>
> - if (vc4_hdmi->hpd_gpio &&
> - gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio)) {
> - connected = true;
> + if (vc4_hdmi->hpd_gpio) {
> + if (gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio))
> + connected = true;
> } else {
> unsigned long flags;
> u32 hotplug;
> --
> 2.31.1
>
Re: [PATCH 20/29] drm/i915/gvt: devirtualize ->{enable,disable}_page_track
On Tue, Nov 02, 2021 at 08:05:52AM +0100, Christoph Hellwig wrote: > Just call the kvmgt functions directly. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gvt.h| 3 +++ > drivers/gpu/drm/i915/gvt/hypercall.h | 2 -- > drivers/gpu/drm/i915/gvt/kvmgt.c | 6 ++ > drivers/gpu/drm/i915/gvt/mpt.h| 28 --- > drivers/gpu/drm/i915/gvt/page_track.c | 8 > 5 files changed, 9 insertions(+), 38 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 19/29] drm/i915/gvt: devirtualize ->gfn_to_mfn
On Tue, Nov 02, 2021 at 08:05:51AM +0100, Christoph Hellwig wrote: > Just open code it in the only caller. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gtt.c | 9 + > drivers/gpu/drm/i915/gvt/hypercall.h | 1 - > drivers/gpu/drm/i915/gvt/kvmgt.c | 16 > drivers/gpu/drm/i915/gvt/mpt.h | 14 -- > 4 files changed, 5 insertions(+), 35 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 18/29] drm/i915/gvt: devirtualize ->is_valid_gfn
On Tue, Nov 02, 2021 at 08:05:50AM +0100, Christoph Hellwig wrote: > Just call the code directly and move towards the callers. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gtt.c | 20 ++-- > drivers/gpu/drm/i915/gvt/hypercall.h | 1 - > drivers/gpu/drm/i915/gvt/kvmgt.c | 17 - > drivers/gpu/drm/i915/gvt/mpt.h | 17 - > 4 files changed, 18 insertions(+), 37 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [Intel-gfx] [PATCH 11/28] drm/i915/pm: Move CONTEXT_VALID_BIT check
On Thu, 21 Oct 2021 at 11:37, Maarten Lankhorst
wrote:
>
> Resetting will clear the CONTEXT_VALID_BIT, so wait until after that to test.
>
AFAIK this seems to be fixing something earlier in the series(maybe
patch 7?) i.e without this patch we seem to trigger the BUG_ON. If so,
this needs to be much earlier in the series?
Also this probably needs some more commentary in the commit message
for whether moving the BUG_ON matters, or if this is potentially
papering over something significant?
> Signed-off-by: Maarten Lankhorst
> Reviewed-by: Niranjana Vishwanathapura
> ---
> drivers/gpu/drm/i915/gt/intel_engine_pm.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> index a1334b48dde7..849fbb229bd3 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> @@ -52,8 +52,6 @@ static int __engine_unpark(struct intel_wakeref *wf)
> /* Discard stale context state from across idling */
> ce = engine->kernel_context;
> if (ce) {
> - GEM_BUG_ON(test_bit(CONTEXT_VALID_BIT, >flags));
> -
> /* Flush all pending HW writes before we touch the context */
> while (unlikely(intel_context_inflight(ce)))
> intel_engine_flush_submission(engine);
> @@ -68,6 +66,9 @@ static int __engine_unpark(struct intel_wakeref *wf)
> ce->timeline->seqno,
> READ_ONCE(*ce->timeline->hwsp_seqno),
> ce->ring->emit);
> +
> + GEM_BUG_ON(test_bit(CONTEXT_VALID_BIT, >flags));
> +
> GEM_BUG_ON(ce->timeline->seqno !=
>READ_ONCE(*ce->timeline->hwsp_seqno));
> }
> --
> 2.33.0
>
Re: [PATCH 17/29] drm/i915/gvt: devirtualize ->inject_msi
On Tue, Nov 02, 2021 at 08:05:49AM +0100, Christoph Hellwig wrote: > Just open code the MSI injection in a single place instead of going > through the method table. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/hypercall.h | 1 - > drivers/gpu/drm/i915/gvt/interrupt.c | 38 +++- > drivers/gpu/drm/i915/gvt/kvmgt.c | 24 -- > drivers/gpu/drm/i915/gvt/mpt.h | 37 --- > 4 files changed, 37 insertions(+), 63 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 16/29] drm/i915/gvt: devirtualize ->detach_vgpu
On Tue, Nov 02, 2021 at 08:05:48AM +0100, Christoph Hellwig wrote: > Just call the function directly. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gvt.h | 1 + > drivers/gpu/drm/i915/gvt/hypercall.h | 1 - > drivers/gpu/drm/i915/gvt/kvmgt.c | 3 +-- > drivers/gpu/drm/i915/gvt/mpt.h | 16 > drivers/gpu/drm/i915/gvt/vgpu.c | 2 +- > 5 files changed, 3 insertions(+), 20 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 15/29] drm/i915/gvt: devirtualize ->set_edid and ->set_opregion
On Tue, Nov 02, 2021 at 08:05:47AM +0100, Christoph Hellwig wrote: > Just call the code to setup the opregions and EDID data directly. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gvt.h | 3 +++ > drivers/gpu/drm/i915/gvt/hypercall.h | 3 --- > drivers/gpu/drm/i915/gvt/kvmgt.c | 6 ++ > drivers/gpu/drm/i915/gvt/mpt.h | 32 > drivers/gpu/drm/i915/gvt/vgpu.c | 6 +++--- > 5 files changed, 8 insertions(+), 42 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 14/29] drm/i915/gvt: devirtualize ->{get,put}_vfio_device
On Tue, Nov 02, 2021 at 08:05:46AM +0100, Christoph Hellwig wrote: > Just open code the calls to the VFIO APIs. > > Signed-off-by: Christoph Hellwig > drivers/gpu/drm/i915/gvt/dmabuf.c| 12 ++- > drivers/gpu/drm/i915/gvt/hypercall.h | 2 -- > drivers/gpu/drm/i915/gvt/kvmgt.c | 22 > drivers/gpu/drm/i915/gvt/mpt.h | 30 > 4 files changed, 7 insertions(+), 59 deletions(-) I suspect the use of get/put here is very questionable It only prevents the vfio_device from becoming unregistered, but the main thing a mdev should care about is if it is still beween opne_device() / close_device() - ie the FD is open, there is a SW IOMMU available, and memory pins can be made. Still, not for this patch Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 13/29] drm/i915/gvt: devirtualize ->{read,write}_gpa
On Tue, Nov 02, 2021 at 08:05:45AM +0100, Christoph Hellwig wrote: > Just call the VFIO functions directly instead of through the method > table. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/cmd_parser.c | 4 +-- > drivers/gpu/drm/i915/gvt/execlist.c | 12 - > drivers/gpu/drm/i915/gvt/gtt.c| 6 ++--- > drivers/gpu/drm/i915/gvt/gvt.h| 37 +++ > drivers/gpu/drm/i915/gvt/hypercall.h | 4 --- > drivers/gpu/drm/i915/gvt/kvmgt.c | 23 - > drivers/gpu/drm/i915/gvt/mmio.c | 4 +-- > drivers/gpu/drm/i915/gvt/mpt.h| 32 --- > drivers/gpu/drm/i915/gvt/opregion.c | 10 +++- > drivers/gpu/drm/i915/gvt/scheduler.c | 37 +-- > 10 files changed, 72 insertions(+), 97 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH] drm/i915: fixup dma_fence_wait usage
On 11/2/21 16:50, Matthew Auld wrote:
dma_fence_wait expects a boolean for whether it should be interruptible,
not a timeout value.
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
---
drivers/gpu/drm/i915/i915_vma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 90546fa58fc1..8781c4f61952 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -349,7 +349,7 @@ int i915_vma_wait_for_bind(struct i915_vma *vma)
fence = dma_fence_get_rcu_safe(>active.excl.fence);
rcu_read_unlock();
if (fence) {
- err = dma_fence_wait(fence, MAX_SCHEDULE_TIMEOUT);
+ err = dma_fence_wait(fence, true);
dma_fence_put(fence);
}
}
LGTM. Reviewed-by: Thomas Hellström
Re: [PATCH 12/29] drm/i915/gvt: remove vgpu->handle
On Tue, Nov 02, 2021 at 08:05:44AM +0100, Christoph Hellwig wrote: > Always pass the actual vgpu structure instead of encoding it as a > "handle" and add a bool flag to denote if a VGPU is attached. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gvt.h | 3 +- > drivers/gpu/drm/i915/gvt/hypercall.h | 32 +++ > drivers/gpu/drm/i915/gvt/kvmgt.c | 126 +-- > drivers/gpu/drm/i915/gvt/mpt.h | 20 ++--- > drivers/gpu/drm/i915/gvt/vgpu.c | 6 +- > 5 files changed, 71 insertions(+), 116 deletions(-) Reviewed-by: Jason Gunthorpe Jason
[PATCH] drm/i915: fixup dma_fence_wait usage
dma_fence_wait expects a boolean for whether it should be interruptible,
not a timeout value.
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
---
drivers/gpu/drm/i915/i915_vma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 90546fa58fc1..8781c4f61952 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -349,7 +349,7 @@ int i915_vma_wait_for_bind(struct i915_vma *vma)
fence = dma_fence_get_rcu_safe(>active.excl.fence);
rcu_read_unlock();
if (fence) {
- err = dma_fence_wait(fence, MAX_SCHEDULE_TIMEOUT);
+ err = dma_fence_wait(fence, true);
dma_fence_put(fence);
}
}
--
2.31.1
Re: [PATCH 11/29] drm/i915/gvt: merge struct kvmgt_guest_info into strut intel_vgpu
On Tue, Nov 02, 2021 at 08:05:43AM +0100, Christoph Hellwig wrote: > Consolidate the per-VGPU structures into a single one. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gvt.h | 8 +++ > drivers/gpu/drm/i915/gvt/kvmgt.c | 117 --- > 2 files changed, 52 insertions(+), 73 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 10/29] drm/i915/gvt: merge struct kvmgt_vdev into struct intel_vgpu
On Tue, Nov 02, 2021 at 08:05:42AM +0100, Christoph Hellwig wrote: > Move towards having only a single structure for the per-VGPU state. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gvt.h | 31 ++- > drivers/gpu/drm/i915/gvt/hypercall.h | 1 - > drivers/gpu/drm/i915/gvt/kvmgt.c | 288 ++- > drivers/gpu/drm/i915/gvt/mpt.h | 16 -- > drivers/gpu/drm/i915/gvt/vgpu.c | 8 +- > 5 files changed, 128 insertions(+), 216 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 09/29] drm/i915/gvt: remove the unused from_virt_to_mfn op
On Tue, Nov 02, 2021 at 08:05:41AM +0100, Christoph Hellwig wrote: > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/hypercall.h | 1 - > drivers/gpu/drm/i915/gvt/kvmgt.c | 6 -- > drivers/gpu/drm/i915/gvt/mpt.h | 12 > 3 files changed, 19 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 08/29] drm/i915/gvt: remove the map_gfn_to_mfn and set_trap_area ops
On Tue, Nov 02, 2021 at 08:05:40AM +0100, Christoph Hellwig wrote: > The map_gfn_to_mfn and set_trap_area ops are never defined, so remove > them and clean up code that depends on them in the callers. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/cfg_space.c | 89 ++-- > drivers/gpu/drm/i915/gvt/hypercall.h | 4 -- > drivers/gpu/drm/i915/gvt/mpt.h | 44 -- > 3 files changed, 17 insertions(+), 120 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 07/29] drm/i915/gvt: remove intel_gvt_ops
On Tue, Nov 02, 2021 at 08:05:39AM +0100, Christoph Hellwig wrote: > Remove these pointless indirect alls by just calling the only instance > of each method directly. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gvt.c | 20 +-- > drivers/gpu/drm/i915/gvt/gvt.h | 24 -- > drivers/gpu/drm/i915/gvt/hypercall.h | 2 +- > drivers/gpu/drm/i915/gvt/kvmgt.c | 37 +++- > drivers/gpu/drm/i915/gvt/mpt.h | 5 ++-- > 5 files changed, 19 insertions(+), 69 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH v4] backlight: lp855x: Switch to atomic PWM API
On Mon, Nov 01, 2021 at 10:19:21AM -0300, Maíra Canal wrote:
> Remove legacy PWM interface (pwm_config, pwm_enable, pwm_disable) and
> replace it for the atomic PWM API.
>
> Signed-off-by: Maíra Canal
> ---
> V1 -> V2: Initialize variable and simply conditional loop
> V2 -> V3: Fix assignment of NULL variable
> V3 -> V4: Replace division for pwm_set_relative_duty_cycle
> ---
> drivers/video/backlight/lp855x_bl.c | 20
> 1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/video/backlight/lp855x_bl.c
> b/drivers/video/backlight/lp855x_bl.c
> index e94932c69f54..bbf24564082a 100644
> --- a/drivers/video/backlight/lp855x_bl.c
> +++ b/drivers/video/backlight/lp855x_bl.c
> @@ -233,9 +233,8 @@ static int lp855x_configure(struct lp855x *lp)
>
> static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
> {
> - unsigned int period = lp->pdata->period_ns;
> - unsigned int duty = br * period / max_br;
> struct pwm_device *pwm;
> + struct pwm_state state;
>
> /* request pwm device with the consumer name */
> if (!lp->pwm) {
> @@ -245,18 +244,15 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br,
> int max_br)
>
> lp->pwm = pwm;
>
> - /*
> - * FIXME: pwm_apply_args() should be removed when switching to
> - * the atomic PWM API.
> - */
> - pwm_apply_args(pwm);
> + pwm_init_state(lp->pwm, );
> + state.period = lp->pdata->period_ns;
> }
>
> - pwm_config(lp->pwm, duty, period);
> - if (duty)
> - pwm_enable(lp->pwm);
> - else
> - pwm_disable(lp->pwm);
> + pwm_get_state(lp->pwm, );
Should this be:
} else {
pwm_get_state(lp->pwm, );
}
As currently written this will clobber the state.period that was set
above.
Daniel.
> + pwm_set_relative_duty_cycle(, br, max_br);
> + state.enabled = state.duty_cycle;
> +
> + pwm_apply_state(lp->pwm, );
> }
>
> static int lp855x_bl_update_status(struct backlight_device *bl)
> --
> 2.31.1
>
Re: [PATCH] drm/virtio: delay pinning the pages till first use
On Tue, Nov 2, 2021 at 6:07 AM Gerd Hoffmann wrote: > > On Tue, Nov 02, 2021 at 12:31:39PM +0100, Maksym Wezdecki wrote: > > From: mwezdeck > > > > The idea behind the commit: > > 1. not pin the pages during resource_create ioctl > > 2. pin the pages on the first use during: > > - transfer_*_host ioctl > > - map ioctl > > i.e. basically lazy pinning. Approach looks sane to me. > > > 3. introduce new ioctl for pinning pages on demand > > What is the use case for this ioctl? > In any case this should be a separate patch. Lazy pinning can be a nice optimization that userspace does not necessarily need to know about. This patch however skips pinning for execbuffer ioctl and introduces a new pin ioctl instead. That is a red flag.
Re: [PATCH 05/29] drm/i915/gvt: rename intel_vgpu_ops to intel_vgpu_mdev_ops
On Tue, Nov 02, 2021 at 08:05:37AM +0100, Christoph Hellwig wrote: > Free the intel_vgpu_ops symbol name for something that fits better. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/kvmgt.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 04/29] drm/i915/gvt: remove enum hypervisor_type
On Tue, Nov 02, 2021 at 08:05:36AM +0100, Christoph Hellwig wrote: > The only supported hypervisor is KVM, so don't bother with dead code > enumerating hypervisors. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/gvt/gvt.c | 17 +-- > drivers/gpu/drm/i915/gvt/gvt.h | 1 - > drivers/gpu/drm/i915/gvt/hypercall.h | 6 -- > drivers/gpu/drm/i915/gvt/kvmgt.c | 1 - > drivers/gpu/drm/i915/gvt/opregion.c | 150 ++- > 5 files changed, 34 insertions(+), 141 deletions(-) Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH 03/29] drm/i915/gvt: remove module refcounting in intel_gvt_{,un}register_hypervisor
On Tue, Nov 02, 2021 at 08:05:35AM +0100, Christoph Hellwig wrote: > THIS_MODULE always is reference when a symbol called by it is used, so > don't bother with the additional reference. heh, these functions are only called from a module init/exit even Reviewed-by: Jason Gunthorpe Jason
Re: [PATCH] drm/sun4i: fix unmet dependency on RESET_CONTROLLER for PHY_SUN6I_MIPI_DPHY
Hi, On Thu, Oct 28, 2021 at 06:13:27PM -0400, Julian Braha wrote: > When PHY_SUN6I_MIPI_DPHY is selected, and RESET_CONTROLLER > is not selected, Kbuild gives the following warning: > > WARNING: unmet direct dependencies detected for PHY_SUN6I_MIPI_DPHY > Depends on [n]: (ARCH_SUNXI [=n] || COMPILE_TEST [=y]) && HAS_IOMEM [=y] && > COMMON_CLK [=y] && RESET_CONTROLLER [=n] > Selected by [y]: > - DRM_SUN6I_DSI [=y] && HAS_IOMEM [=y] && DRM_SUN4I [=y] > > This is because DRM_SUN6I_DSI selects PHY_SUN6I_MIPI_DPHY > without selecting or depending on RESET_CONTROLLER, despite > PHY_SUN6I_MIPI_DPHY depending on RESET_CONTROLLER. > > These unmet dependency bugs were detected by Kismet, > a static analysis tool for Kconfig. Please advise if this > is not the appropriate solution. > > Signed-off-by: Julian Braha > --- > drivers/gpu/drm/sun4i/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig > index 5755f0432e77..3f70a14a3c94 100644 > --- a/drivers/gpu/drm/sun4i/Kconfig > +++ b/drivers/gpu/drm/sun4i/Kconfig > @@ -46,6 +46,7 @@ config DRM_SUN6I_DSI > default MACH_SUN8I > select CRC_CCITT > select DRM_MIPI_DSI > + select RESET_CONTROLLER The indentation is off, can you fix it and resend it? Maxime signature.asc Description: PGP signature
Re: [PATCH -V2] drm/sun4i: Grab reference of connector before return connector from sun4i_tcon_get_connector
Hi,
On Tue, Nov 02, 2021 at 04:46:28AM -0400, He Ying wrote:
> From the comments of drm_for_each_connector_iter(), we know
> that "connector is only valid within the list body, if you
> want to use connector after calling drm_connector_list_iter_end()
> then you need to grab your own reference first using
> drm_connector_get()". So fix the wrong use of connector
> according to the comments and then call drm_connector_put()
> after using connector finishes.
>
> Signed-off-by: He Ying
> ---
>
> V2:
> Use proper subject prefix
>
> drivers/gpu/drm/sun4i/sun4i_tcon.c | 18 +-
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index 9f06dec0fc61..24fa6784ee5f 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -47,12 +47,12 @@ static struct drm_connector
> *sun4i_tcon_get_connector(const struct drm_encoder *
> drm_connector_list_iter_begin(encoder->dev, );
> drm_for_each_connector_iter(connector, )
> if (connector->encoder == encoder) {
> - drm_connector_list_iter_end();
> - return connector;
> + drm_connector_get(connector);
> + break;
> }
> drm_connector_list_iter_end();
>
> - return NULL;
> + return connector;
Connector might be uninitialized if we don't find one here
> }
>
> static int sun4i_tcon_get_pixel_depth(const struct drm_encoder *encoder)
> @@ -65,6 +65,7 @@ static int sun4i_tcon_get_pixel_depth(const struct
> drm_encoder *encoder)
> return -EINVAL;
>
> info = >display_info;
> + drm_connector_put(connector);
> if (info->num_bus_formats != 1)
We're still accessing connector->display_info here, but it might have been
freed already.
Maxime
signature.asc
Description: PGP signature
[PATCH 12/13] drm/vc4: hdmi: Leverage new SCDC atomic_check
Now that we have a generic helper to fill the scrambling status, let's
use it.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 32 ++--
drivers/gpu/drm/vc4/vc4_hdmi.h | 6 ++
2 files changed, 16 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 82878718e5fc..aa6700622797 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -297,6 +297,14 @@ static int vc4_hdmi_connector_atomic_check(struct
drm_connector *connector,
struct drm_connector_state *new_state =
drm_atomic_get_new_connector_state(state, connector);
struct drm_crtc *crtc = new_state->crtc;
+ int ret;
+
+ ret = drm_atomic_helper_connector_hdmi_check(connector, state);
+ if (ret)
+ return ret;
+
+ if (new_state->hdmi_needs_scrambling !=
new_state->hdmi_needs_high_tmds_ratio)
+ return -EINVAL;
if (!crtc)
return 0;
@@ -586,37 +594,16 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder
*encoder)
vc4_hdmi_set_hdr_infoframe(encoder);
}
-static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder)
-{
- struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
- struct drm_display_info *display = _hdmi->connector.display_info;
-
- lockdep_assert_held(_hdmi->mutex);
-
- if (!display->is_hdmi)
- return false;
-
- if (!display->hdmi.scdc.supported ||
- !display->hdmi.scdc.scrambling.supported)
- return false;
-
- return true;
-}
-
#define SCRAMBLING_POLLING_DELAY_MS1000
static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder)
{
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
- const struct drm_display_mode *mode = _hdmi->saved_adjusted_mode;
unsigned long flags;
lockdep_assert_held(_hdmi->mutex);
- if (!vc4_hdmi_supports_scrambling(encoder))
- return;
-
- if (!drm_mode_hdmi_requires_scrambling(mode))
+ if (!vc4_hdmi->scdc_needed)
return;
drm_scdc_set_high_tmds_clock_ratio(vc4_hdmi->ddc, true);
@@ -1228,6 +1215,7 @@ static void vc4_hdmi_encoder_atomic_mode_set(struct
drm_encoder *encoder,
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
mutex_lock(_hdmi->mutex);
+ vc4_hdmi->scdc_needed = conn_state->hdmi_needs_scrambling;
memcpy(_hdmi->saved_adjusted_mode,
_state->adjusted_mode,
sizeof(vc4_hdmi->saved_adjusted_mode));
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 460112d68948..1aabc51ede03 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -206,6 +206,12 @@ struct vc4_hdmi {
* the scrambler on? Protected by @mutex.
*/
bool scdc_enabled;
+
+ /**
+* @scdc_needed: Is the HDMI controller needs to have the
+* scrambling on? Protected by @mutex.
+*/
+ bool scdc_needed;
};
static inline struct vc4_hdmi *
--
2.32.0
[PATCH 06/13] drm/vc4: hdmi: Remove unused argument in vc4_hdmi_supports_scrambling
Even though vc4_hdmi_supports_scrambling takes a mode as an argument, it
never uses it. Let's remove it.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index fa76ae2c94e4..49bcb0342cc9 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -568,8 +568,7 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder
*encoder)
vc4_hdmi_set_hdr_infoframe(encoder);
}
-static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder,
-const struct drm_display_mode *mode)
+static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder)
{
struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
@@ -597,7 +596,7 @@ static void vc4_hdmi_enable_scrambling(struct drm_encoder
*encoder)
lockdep_assert_held(_hdmi->mutex);
- if (!vc4_hdmi_supports_scrambling(encoder, mode))
+ if (!vc4_hdmi_supports_scrambling(encoder))
return;
if (!drm_mode_hdmi_requires_scrambling(mode))
--
2.32.0
[PATCH 13/13] drm/vc4: hdmi: Reset link on hotplug
Enabling the scrambling on reconnection seems to work so far but breaks
the HDMI2.0 specification and has introduced some issues in the past
with i915.
Let's do a mode set on the connector instead to follow the
specification.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index aa6700622797..a1f40548dd48 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -175,9 +175,8 @@ static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi
*vc4_hdmi)
static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi) {}
#endif
-static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder);
-
static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
+ struct drm_modeset_acquire_ctx *ctx,
enum drm_connector_status status)
{
struct drm_connector *connector = _hdmi->connector;
@@ -198,7 +197,7 @@ static void vc4_hdmi_handle_hotplug(struct vc4_hdmi
*vc4_hdmi,
cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, edid);
kfree(edid);
- vc4_hdmi_enable_scrambling(_hdmi->encoder.base.base);
+ drm_atomic_helper_connector_hdmi_reset_link(connector, ctx);
}
static int vc4_hdmi_connector_detect_ctx(struct drm_connector *connector,
@@ -236,7 +235,7 @@ static int vc4_hdmi_connector_detect_ctx(struct
drm_connector *connector,
status = connector_status_connected;
}
- vc4_hdmi_handle_hotplug(vc4_hdmi, status);
+ vc4_hdmi_handle_hotplug(vc4_hdmi, ctx, status);
pm_runtime_put(_hdmi->pdev->dev);
return status;
--
2.32.0
[PATCH 11/13] drm/vc4: hdmi: Switch to detect_ctx
We'll need the locking context in future patch, so let's convert .detect
to .detect_ctx.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 2d7c34b306c9..82878718e5fc 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -201,8 +201,9 @@ static void vc4_hdmi_handle_hotplug(struct vc4_hdmi
*vc4_hdmi,
vc4_hdmi_enable_scrambling(_hdmi->encoder.base.base);
}
-static enum drm_connector_status
-vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
+static int vc4_hdmi_connector_detect_ctx(struct drm_connector *connector,
+struct drm_modeset_acquire_ctx *ctx,
+bool force)
{
struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
enum drm_connector_status status = connector_status_disconnected;
@@ -353,7 +354,6 @@ vc4_hdmi_connector_duplicate_state(struct drm_connector
*connector)
}
static const struct drm_connector_funcs vc4_hdmi_connector_funcs = {
- .detect = vc4_hdmi_connector_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
.destroy = vc4_hdmi_connector_destroy,
.reset = vc4_hdmi_connector_reset,
@@ -362,6 +362,7 @@ static const struct drm_connector_funcs
vc4_hdmi_connector_funcs = {
};
static const struct drm_connector_helper_funcs vc4_hdmi_connector_helper_funcs
= {
+ .detect_ctx = vc4_hdmi_connector_detect_ctx,
.get_modes = vc4_hdmi_connector_get_modes,
.atomic_check = vc4_hdmi_connector_atomic_check,
};
--
2.32.0
[PATCH 10/13] drm/vc4: hdmi: Simplify the connector state retrieval
When we have the entire DRM state, retrieving the connector state only
requires the drm_connector pointer. Fortunately for us, we have
allocated it as a part of the vc4_hdmi structure, so we can retrieve get
a pointer by simply accessing our field in that structure.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 21 +++--
1 file changed, 3 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 4f2f138f93e3..2d7c34b306c9 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -996,30 +996,15 @@ static void vc4_hdmi_recenter_fifo(struct vc4_hdmi
*vc4_hdmi)
"VC4_HDMI_FIFO_CTL_RECENTER_DONE");
}
-static struct drm_connector_state *
-vc4_hdmi_encoder_get_connector_state(struct drm_encoder *encoder,
-struct drm_atomic_state *state)
-{
- struct drm_connector_state *conn_state;
- struct drm_connector *connector;
- unsigned int i;
-
- for_each_new_connector_in_state(state, connector, conn_state, i) {
- if (conn_state->best_encoder == encoder)
- return conn_state;
- }
-
- return NULL;
-}
-
static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder,
struct drm_atomic_state *state)
{
+ struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
+ struct drm_connector *connector = _hdmi->connector;
struct drm_connector_state *conn_state =
- vc4_hdmi_encoder_get_connector_state(encoder, state);
+ drm_atomic_get_new_connector_state(state, connector);
struct vc4_hdmi_connector_state *vc4_conn_state =
conn_state_to_vc4_hdmi_conn_state(conn_state);
- struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
struct drm_display_mode *mode = _hdmi->saved_adjusted_mode;
unsigned long pixel_rate = vc4_conn_state->pixel_rate;
unsigned long bvb_rate, hsm_rate;
--
2.32.0
[PATCH 07/13] drm/vc4: hdmi: Remove mutex in detect
We recently introduced a new mutex to protect concurrent execution of
ALSA and KMS hooks, and the concurrent access to some of vc4_hdmi
fields.
However, using it in the detect hook was creating a reentrency issue
with CEC code. Indeed, calling cec_s_phys_addr_from_edid from detect
might call the CEC adap_enable hook with the lock held, eventually
resulting in a deadlock.
Since we didn't really need to protect anything at the moment in the CEC
code, the decision was made to ignore the mutex in those CEC hooks,
working around the issue.
However, we can have the same thing happening if we end up triggering a
mode set from the detect callback, for example using
drm_atomic_helper_connector_hdmi_reset_link().
Since we don't really need to protect anything in detect either, let's
just drop the lock in detect, and add it again in CEC.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 89 +-
drivers/gpu/drm/vc4/vc4_hdmi.h | 10 +---
2 files changed, 36 insertions(+), 63 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 49bcb0342cc9..826ca7aaf8d7 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -183,7 +183,16 @@ vc4_hdmi_connector_detect(struct drm_connector *connector,
bool force)
struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
bool connected = false;
- mutex_lock(_hdmi->mutex);
+ /*
+* NOTE: This function should really take vc4_hdmi->mutex, but
+* doing so results in reentrancy issues since
+* cec_s_phys_addr_from_edid might call .adap_enable, which
+* leads to that funtion being called with our mutex held.
+*
+* Concurrency isn't an issue at the moment since we don't share
+* any state with any of the other frameworks so we can ignore
+* the lock for now.
+*/
WARN_ON(pm_runtime_resume_and_get(_hdmi->pdev->dev));
@@ -215,13 +224,11 @@ vc4_hdmi_connector_detect(struct drm_connector
*connector, bool force)
vc4_hdmi_enable_scrambling(_hdmi->encoder.base.base);
pm_runtime_put(_hdmi->pdev->dev);
- mutex_unlock(_hdmi->mutex);
return connector_status_connected;
}
cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
pm_runtime_put(_hdmi->pdev->dev);
- mutex_unlock(_hdmi->mutex);
return connector_status_disconnected;
}
@@ -238,14 +245,21 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
int ret = 0;
struct edid *edid;
- mutex_lock(_hdmi->mutex);
+ /*
+* NOTE: This function should really take vc4_hdmi->mutex, but
+* doing so results in reentrancy issues since
+* cec_s_phys_addr_from_edid might call .adap_enable, which
+* leads to that funtion being called with our mutex held.
+*
+* Concurrency isn't an issue at the moment since we don't share
+* any state with any of the other frameworks so we can ignore
+* the lock for now.
+*/
edid = drm_get_edid(connector, vc4_hdmi->ddc);
cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, edid);
- if (!edid) {
- ret = -ENODEV;
- goto out;
- }
+ if (!edid)
+ return -ENODEV;
vc4_encoder->hdmi_monitor = drm_detect_hdmi_monitor(edid);
@@ -265,9 +279,6 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
}
}
-out:
- mutex_unlock(_hdmi->mutex);
-
return ret;
}
@@ -1996,21 +2007,12 @@ static int vc4_hdmi_cec_enable(struct cec_adapter *adap)
u32 val;
int ret;
- /*
-* NOTE: This function should really take vc4_hdmi->mutex, but doing so
-* results in a reentrancy since cec_s_phys_addr_from_edid() called in
-* .detect or .get_modes might call .adap_enable, which leads to this
-* function being called with that mutex held.
-*
-* Concurrency is not an issue for the moment since we don't share any
-* state with KMS, so we can ignore the lock for now, but we need to
-* keep it in mind if we were to change that assumption.
-*/
-
ret = pm_runtime_resume_and_get(_hdmi->pdev->dev);
if (ret)
return ret;
+ mutex_lock(_hdmi->mutex);
+
spin_lock_irqsave(_hdmi->hw_lock, flags);
val = HDMI_READ(HDMI_CEC_CNTRL_5);
@@ -2045,6 +2047,8 @@ static int vc4_hdmi_cec_enable(struct cec_adapter *adap)
spin_unlock_irqrestore(_hdmi->hw_lock, flags);
+ mutex_unlock(_hdmi->mutex);
+
return 0;
}
@@ -2053,16 +2057,7 @@ static int vc4_hdmi_cec_disable(struct cec_adapter *adap)
struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap);
unsigned long flags;
- /*
-* NOTE: This function should
[PATCH 04/13] drm/atomic: Add HDMI reset link helper
During a hotplug cycle (such as a TV going out of suspend, or when the
cable is disconnected and reconnected), the expectation is that the same
state used before the disconnection is reused until the next commit.
However, the HDMI scrambling requires that some flags are set in the
monitor, and those flags are very likely to be reset when the cable has
been disconnected. This will thus result in a blank display, even if the
display pipeline configuration hasn't been modified or is in the exact
same state.
One solution would be to enable the scrambling-related bits again on
reconnection, but the HDMI 2.0 specification (Section 6.1.3.1 -
Scrambling Control) requires that the scrambling enable bit is set
before sending any scrambled video signal. Using that solution would
break that specification expectation.
Thus, we need to do a full modeset on the connector so that we disable
the video signal, enable the scrambling bit, and enable the video signal
again.
The i915 code was doing this already, so let's take its code and
convert it into a generic helper.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_atomic_helper.c | 109
include/drm/drm_atomic_helper.h | 3 +
2 files changed, 112 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 2c0c6ec92820..9f3fcc65e66e 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -38,6 +38,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -3524,3 +3525,111 @@ drm_atomic_helper_bridge_propagate_bus_fmt(struct
drm_bridge *bridge,
return input_fmts;
}
EXPORT_SYMBOL(drm_atomic_helper_bridge_propagate_bus_fmt);
+
+static int modeset_pipe(struct drm_crtc *crtc,
+ struct drm_modeset_acquire_ctx *ctx)
+{
+ struct drm_atomic_state *state;
+ struct drm_crtc_state *crtc_state;
+ int ret;
+
+ state = drm_atomic_state_alloc(crtc->dev);
+ if (!state)
+ return -ENOMEM;
+
+ state->acquire_ctx = ctx;
+
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state)) {
+ ret = PTR_ERR(crtc_state);
+ goto out;
+ }
+
+ crtc_state->connectors_changed = true;
+
+ ret = drm_atomic_commit(state);
+out:
+ drm_atomic_state_put(state);
+
+ return ret;
+}
+
+/**
+ * drm_atomic_helper_connector_hdmi_reset_link() - Resets an HDMI link
+ * @connector: DRM connector we want to reset
+ * @ctx: Lock acquisition context
+ *
+ * This helper is here to restore the HDMI link state after the
+ * connector status has changed, typically when a TV has come out of
+ * suspend or when the HDMI cable has been disconnected and then
+ * reconnected.
+ *
+ * Returns:
+ * 0 on success, a negative error code otherwise.
+ */
+int drm_atomic_helper_connector_hdmi_reset_link(struct drm_connector
*connector,
+ struct drm_modeset_acquire_ctx
*ctx)
+{
+ struct drm_device *drm = connector->dev;
+ struct drm_connector_state *conn_state;
+ struct drm_crtc_state *crtc_state;
+ struct drm_crtc *crtc;
+ u8 config;
+ int ret;
+
+ if (!connector)
+ return 0;
+
+ drm_WARN_ON(drm,
+ (connector->connector_type != DRM_MODE_CONNECTOR_HDMIA) &&
+ (connector->connector_type != DRM_MODE_CONNECTOR_HDMIB));
+
+ ret = drm_modeset_lock(>mode_config.connection_mutex, ctx);
+ if (ret)
+ return ret;
+
+ conn_state = connector->state;
+ crtc = conn_state->crtc;
+ if (!crtc)
+ return 0;
+
+ ret = drm_modeset_lock(>mutex, ctx);
+ if (ret)
+ return ret;
+
+ crtc_state = crtc->state;
+ if (!crtc_state->active)
+ return 0;
+
+ if (!conn_state->hdmi_needs_high_tmds_ratio &&
+ !conn_state->hdmi_needs_scrambling)
+ return 0;
+
+ if (conn_state->commit &&
+ !try_wait_for_completion(_state->commit->hw_done))
+ return 0;
+
+ ret = drm_scdc_readb(connector->ddc, SCDC_TMDS_CONFIG, );
+ if (ret < 0) {
+ drm_err(drm, "Failed to read TMDS config: %d\n", ret);
+ return 0;
+ }
+
+ if (!!(config & SCDC_TMDS_BIT_CLOCK_RATIO_BY_40) ==
+ conn_state->hdmi_needs_high_tmds_ratio &&
+ !!(config & SCDC_SCRAMBLING_ENABLE) ==
+ conn_state->hdmi_needs_scrambling)
+ return 0;
+
+ /*
+* HDMI 2.0 says that one should not send scrambled data
+* prior to configuring the sink scrambling, and that
+* TMDS clock/data transmission should be suspended when
+* changing the TMDS clock rate in the sink. So let's
+* just do a full modeset here, even though some sinks
+* would be perfectly happy if
[PATCH 05/13] drm/scdc: Document hotplug gotchas
There's some interactions between the SCDC setup and the disconnection / reconnection of displays. Let's document it and a solution. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_scdc_helper.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/gpu/drm/drm_scdc_helper.c b/drivers/gpu/drm/drm_scdc_helper.c index 48a382464d54..033a9e407acb 100644 --- a/drivers/gpu/drm/drm_scdc_helper.c +++ b/drivers/gpu/drm/drm_scdc_helper.c @@ -34,6 +34,19 @@ * HDMI 2.0 specification. It is a point-to-point protocol that allows the * HDMI source and HDMI sink to exchange data. The same I2C interface that * is used to access EDID serves as the transport mechanism for SCDC. + * + * Note: The SCDC status is going to be lost when the display is + * disconnected. This can happen physically when the user disconnects + * the cable, but also when a display is switched on (such as waking up + * a TV). + * + * This is further complicated by the fact that, upon a disconnection / + * reconnection, KMS won't change the mode on its own. This means that + * one can't just rely on setting the SCDC status on enable, but also + * has to track the connector status changes using interrupts and + * restore the SCDC status. The typical solution for this is to call + * drm_atomic_helper_connector_hdmi_reset_link() in + * drm_connector_helper_funcs.detect_ctx(). */ #define SCDC_I2C_SLAVE_ADDRESS 0x54 -- 2.32.0
[PATCH 08/13] drm/vc4: hdmi: Remove HDMI flag from encoder
The hdmi_monitor flag in the vc4_hdmi_encoder structure is redundant
with the is_hdmi flag in the drm_display_info structure.
Let's convert all the users.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 16 ++--
drivers/gpu/drm/vc4/vc4_hdmi.h | 1 -
2 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 826ca7aaf8d7..288c2bfbf88b 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -217,7 +217,6 @@ vc4_hdmi_connector_detect(struct drm_connector *connector,
bool force)
if (edid) {
cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap,
edid);
- vc4_hdmi->encoder.hdmi_monitor =
drm_detect_hdmi_monitor(edid);
kfree(edid);
}
}
@@ -241,7 +240,6 @@ static void vc4_hdmi_connector_destroy(struct drm_connector
*connector)
static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
{
struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
- struct vc4_hdmi_encoder *vc4_encoder = _hdmi->encoder;
int ret = 0;
struct edid *edid;
@@ -261,8 +259,6 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
if (!edid)
return -ENODEV;
- vc4_encoder->hdmi_monitor = drm_detect_hdmi_monitor(edid);
-
drm_connector_update_edid_property(connector, edid);
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
@@ -581,13 +577,12 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder
*encoder)
static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder)
{
- struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
struct drm_display_info *display = _hdmi->connector.display_info;
lockdep_assert_held(_hdmi->mutex);
- if (!vc4_encoder->hdmi_monitor)
+ if (!display->is_hdmi)
return false;
if (!display->hdmi.scdc.supported ||
@@ -1120,11 +1115,12 @@ static void vc4_hdmi_encoder_pre_crtc_enable(struct
drm_encoder *encoder,
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
struct drm_display_mode *mode = _hdmi->saved_adjusted_mode;
struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
+ struct drm_display_info *display = _hdmi->connector.display_info;
unsigned long flags;
mutex_lock(_hdmi->mutex);
- if (vc4_encoder->hdmi_monitor &&
+ if (display->is_hdmi &&
drm_default_rgb_quant_range(mode) ==
HDMI_QUANTIZATION_RANGE_LIMITED) {
if (vc4_hdmi->variant->csc_setup)
vc4_hdmi->variant->csc_setup(vc4_hdmi, true);
@@ -1149,7 +1145,7 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct
drm_encoder *encoder,
{
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
struct drm_display_mode *mode = _hdmi->saved_adjusted_mode;
- struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
+ struct drm_display_info *display = _hdmi->connector.display_info;
bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC;
bool vsync_pos = mode->flags & DRM_MODE_FLAG_PVSYNC;
unsigned long flags;
@@ -1170,7 +1166,7 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct
drm_encoder *encoder,
HDMI_WRITE(HDMI_VID_CTL,
HDMI_READ(HDMI_VID_CTL) & ~VC4_HD_VID_CTL_BLANKPIX);
- if (vc4_encoder->hdmi_monitor) {
+ if (display->is_hdmi) {
HDMI_WRITE(HDMI_SCHEDULER_CONTROL,
HDMI_READ(HDMI_SCHEDULER_CONTROL) |
VC4_HDMI_SCHEDULER_CONTROL_MODE_HDMI);
@@ -1197,7 +1193,7 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct
drm_encoder *encoder,
"!VC4_HDMI_SCHEDULER_CONTROL_HDMI_ACTIVE\n");
}
- if (vc4_encoder->hdmi_monitor) {
+ if (display->is_hdmi) {
spin_lock_irqsave(_hdmi->hw_lock, flags);
WARN_ON(!(HDMI_READ(HDMI_SCHEDULER_CONTROL) &
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 32b0aa447717..460112d68948 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -11,7 +11,6 @@
/* VC4 HDMI encoder KMS struct */
struct vc4_hdmi_encoder {
struct vc4_encoder base;
- bool hdmi_monitor;
bool limited_rgb_range;
};
--
2.32.0
[PATCH 09/13] drm/vc4: hdmi: Simplify the hotplug handling
Our detect callback has a bunch of operations to perform depending on
the current and last status of the connector, such a setting the CEC
physical address or enabling the scrambling again.
This is currently dealt with a bunch of if / else statetements that make
it fairly difficult to read and extend.
Let's move all that logic to a function of its own.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 50 --
1 file changed, 30 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 288c2bfbf88b..4f2f138f93e3 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -177,11 +177,35 @@ static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi
*vc4_hdmi) {}
static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder);
+static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
+ enum drm_connector_status status)
+{
+ struct drm_connector *connector = _hdmi->connector;
+ struct edid *edid;
+
+ if (status == connector->status)
+ return;
+
+ if (status == connector_status_disconnected) {
+ cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
+ return;
+ }
+
+ edid = drm_get_edid(connector, vc4_hdmi->ddc);
+ if (!edid)
+ return;
+
+ cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, edid);
+ kfree(edid);
+
+ vc4_hdmi_enable_scrambling(_hdmi->encoder.base.base);
+}
+
static enum drm_connector_status
vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
{
struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
- bool connected = false;
+ enum drm_connector_status status = connector_status_disconnected;
/*
* NOTE: This function should really take vc4_hdmi->mutex, but
@@ -198,7 +222,7 @@ vc4_hdmi_connector_detect(struct drm_connector *connector,
bool force)
if (vc4_hdmi->hpd_gpio) {
if (gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio))
- connected = true;
+ status = connector_status_connected;
} else {
unsigned long flags;
u32 hotplug;
@@ -208,27 +232,13 @@ vc4_hdmi_connector_detect(struct drm_connector
*connector, bool force)
spin_unlock_irqrestore(_hdmi->hw_lock, flags);
if (hotplug & VC4_HDMI_HOTPLUG_CONNECTED)
- connected = true;
+ status = connector_status_connected;
}
- if (connected) {
- if (connector->status != connector_status_connected) {
- struct edid *edid = drm_get_edid(connector,
vc4_hdmi->ddc);
-
- if (edid) {
- cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap,
edid);
- kfree(edid);
- }
- }
-
- vc4_hdmi_enable_scrambling(_hdmi->encoder.base.base);
- pm_runtime_put(_hdmi->pdev->dev);
- return connector_status_connected;
- }
-
- cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
+ vc4_hdmi_handle_hotplug(vc4_hdmi, status);
pm_runtime_put(_hdmi->pdev->dev);
- return connector_status_disconnected;
+
+ return status;
}
static void vc4_hdmi_connector_destroy(struct drm_connector *connector)
--
2.32.0
[PATCH 03/13] drm/atomic: Add HDMI scrambler state helper
All the drivers that implement the HDMI scrambling setup (dw-hdmi, i915,
tegra, vc4) duplicate the same logic to see if the TMDS ratio or the
scrambling state needs to be modified depending on the current connector
state and CRTC mode.
Since it's basically the same logic everywhere, let's put these two
informations in the connector state, and filled by a atomic_check helper
so that drivers can just use it.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_atomic_state_helper.c | 58 +++
include/drm/drm_atomic_state_helper.h | 3 ++
include/drm/drm_connector.h | 25 ++
3 files changed, 86 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
b/drivers/gpu/drm/drm_atomic_state_helper.c
index ddcf5c2c8e6a..93f40f2975c3 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -454,6 +454,64 @@ void drm_atomic_helper_connector_tv_reset(struct
drm_connector *connector)
}
EXPORT_SYMBOL(drm_atomic_helper_connector_tv_reset);
+/**
+ * drm_atomic_helper_connector_hdmi_check - Checks the state of an HDMI
connector
+ * @connector: DRM connector
+ * @state: DRM atomic state to check
+ *
+ * Checks that an HDMI connector state is sane, and sets the various
+ * HDMI-specific flags in drm_connector_state related to HDMI support.
+ *
+ * Returns:
+ * 0 on success, a negative error code otherwise.
+ */
+int drm_atomic_helper_connector_hdmi_check(struct drm_connector *connector,
+ struct drm_atomic_state *state)
+{
+ struct drm_connector_state *conn_state =
drm_atomic_get_new_connector_state(state,
+
connector);
+ struct drm_display_info *info = >display_info;
+ const struct drm_display_mode *mode;
+ struct drm_crtc_state *crtc_state;
+ struct drm_crtc *crtc;
+ bool required;
+
+ if (!conn_state)
+ return -EINVAL;
+
+ crtc = conn_state->crtc;
+ if (!crtc)
+ return -EINVAL;
+
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state))
+ return PTR_ERR(crtc_state);
+
+ mode = _state->mode;
+ crtc_state->connectors_changed = true;
+ conn_state->hdmi_needs_scrambling = false;
+ conn_state->hdmi_needs_high_tmds_ratio = false;
+
+ if (!info->is_hdmi)
+ return 0;
+
+ if (!info->hdmi.scdc.supported)
+ return 0;
+
+ required = drm_mode_hdmi_requires_scrambling(mode);
+ if (required && !info->hdmi.scdc.scrambling.supported)
+ return -EINVAL;
+
+ if (info->hdmi.scdc.scrambling.low_rates || required)
+ conn_state->hdmi_needs_scrambling = true;
+
+ if (required)
+ conn_state->hdmi_needs_high_tmds_ratio = true;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_atomic_helper_connector_hdmi_check);
+
/**
* __drm_atomic_helper_connector_duplicate_state - copy atomic connector state
* @connector: connector object
diff --git a/include/drm/drm_atomic_state_helper.h
b/include/drm/drm_atomic_state_helper.h
index 3f8f1d627f7c..3d3d1ff355f4 100644
--- a/include/drm/drm_atomic_state_helper.h
+++ b/include/drm/drm_atomic_state_helper.h
@@ -26,6 +26,7 @@
#include
+struct drm_atomic_state;
struct drm_bridge;
struct drm_bridge_state;
struct drm_crtc;
@@ -71,6 +72,8 @@ void __drm_atomic_helper_connector_reset(struct drm_connector
*connector,
struct drm_connector_state
*conn_state);
void drm_atomic_helper_connector_reset(struct drm_connector *connector);
void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector);
+int drm_atomic_helper_connector_hdmi_check(struct drm_connector *connector,
+ struct drm_atomic_state *state);
void
__drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector,
struct drm_connector_state *state);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 030636635af1..78d3d6c78fcb 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -832,6 +832,31 @@ struct drm_connector_state {
* DRM blob property for HDR output metadata
*/
struct drm_property_blob *hdr_output_metadata;
+
+ /**
+* @hdmi_needs_scrambling:
+*
+* Only relevant for HDMI sink. Tracks whether the scrambling
+* should be turned on for the current sink and mode.
+*
+* Drivers needing this should use
+* drm_atomic_helper_connector_hdmi_check() and use the value
+* set here to enable or disable their scrambler.
+*/
+ bool hdmi_needs_scrambling;
+
+ /**
+* @hdmi_needs_high_tmds_ratio:
+*
+* Only relevant for HDMI sink.
[PATCH 01/13] drm/connector: Add define for HDMI 1.4 Maximum Pixel Rate
A lot of drivers open-code the HDMI 1.4 maximum pixel rate in their
driver to test whether the resolutions are supported or if the
scrambling needs to be enabled.
Let's create a common define for everyone to use it.
Cc: Alex Deucher
Cc: amd-...@lists.freedesktop.org
Cc: Andrzej Hajda
Cc: Benjamin Gaignard
Cc: "Christian König"
Cc: Emma Anholt
Cc: intel-...@lists.freedesktop.org
Cc: Jani Nikula
Cc: Jernej Skrabec
Cc: Jerome Brunet
Cc: Jonas Karlman
Cc: Jonathan Hunter
Cc: Joonas Lahtinen
Cc: Kevin Hilman
Cc: Laurent Pinchart
Cc: linux-amlo...@lists.infradead.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-te...@vger.kernel.org
Cc: Martin Blumenstingl
Cc: Neil Armstrong
Cc: "Pan, Xinhui"
Cc: Robert Foss
Cc: Rodrigo Vivi
Cc: Thierry Reding
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 ++--
drivers/gpu/drm/drm_edid.c | 2 +-
drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
drivers/gpu/drm/meson/meson_dw_hdmi.c | 4 ++--
drivers/gpu/drm/radeon/radeon_encoders.c | 2 +-
drivers/gpu/drm/sti/sti_hdmi_tx3g4c28phy.c | 2 +-
drivers/gpu/drm/tegra/sor.c| 8
drivers/gpu/drm/vc4/vc4_hdmi.c | 4 ++--
include/drm/drm_connector.h| 2 ++
9 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 62ae63565d3a..3a58db357be0 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -46,7 +46,7 @@
/* DW-HDMI Controller >= 0x200a are at least compliant with SCDC version 1 */
#define SCDC_MIN_SOURCE_VERSION0x1
-#define HDMI14_MAX_TMDSCLK 34000
+#define HDMI14_MAX_TMDSCLK (DRM_HDMI_14_MAX_TMDS_CLK_KHZ * 1000)
enum hdmi_datamap {
RGB444_8B = 0x01,
@@ -1264,7 +1264,7 @@ static bool dw_hdmi_support_scdc(struct dw_hdmi *hdmi,
* for low rates is not supported either
*/
if (!display->hdmi.scdc.scrambling.low_rates &&
- display->max_tmds_clock <= 34)
+ display->max_tmds_clock <= DRM_HDMI_14_MAX_TMDS_CLK_KHZ)
return false;
return true;
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 7aa2a56a71c8..ec8fb2d098ae 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4966,7 +4966,7 @@ static void drm_parse_hdmi_forum_vsdb(struct
drm_connector *connector,
u32 max_tmds_clock = hf_vsdb[5] * 5000;
struct drm_scdc *scdc = >scdc;
- if (max_tmds_clock > 34) {
+ if (max_tmds_clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ) {
display->max_tmds_clock = max_tmds_clock;
DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n",
display->max_tmds_clock);
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index d2e61f6c6e08..0666203d52b7 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2226,7 +2226,7 @@ int intel_hdmi_compute_config(struct intel_encoder
*encoder,
if (scdc->scrambling.low_rates)
pipe_config->hdmi_scrambling = true;
- if (pipe_config->port_clock > 34) {
+ if (pipe_config->port_clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ) {
pipe_config->hdmi_scrambling = true;
pipe_config->hdmi_high_tmds_clock_ratio = true;
}
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c
b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 0afbd1e70bfc..8078667aea0e 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -434,7 +434,7 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void
*data,
readl_relaxed(priv->io_base + _REG(VPU_HDMI_SETTING));
DRM_DEBUG_DRIVER("\"%s\" div%d\n", mode->name,
-mode->clock > 34 ? 40 : 10);
+mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ ? 40 : 10);
/* Enable clocks */
regmap_update_bits(priv->hhi, HHI_HDMI_CLK_CNTL, 0x, 0x100);
@@ -457,7 +457,7 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void
*data,
dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_BIST_CNTL, BIT(12));
/* TMDS pattern setup */
- if (mode->clock > 34 &&
+ if (mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ &&
dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_YUV8_1X24) {
dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_01,
0);
diff --git a/drivers/gpu/drm/radeon/radeon_encoders.c
b/drivers/gpu/drm/radeon/radeon_encoders.c
index 46549d5179ee..ddd8100e699f 100644
--- a/drivers/gpu/drm/radeon/radeon_encoders.c
+++
[PATCH 02/13] drm/connector: Add helper to check if a mode requires scrambling
Most drivers supporting the HDMI scrambling seem to have the HDMI 1.4
maximum frequency open-coded, and a function to test whether a display
mode is above that threshold to control whether or not scrambling should
be enabled.
Let's create a common define and helper for drivers to reuse.
Cc: Emma Anholt
Cc: Jonathan Hunter
Cc: linux-te...@vger.kernel.org
Cc: Thierry Reding
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/tegra/sor.c| 4 ++--
drivers/gpu/drm/vc4/vc4_hdmi.c | 21 +
include/drm/drm_modes.h| 15 +++
3 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 99a2d627bfeb..390bd04b0d22 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -2192,11 +2192,11 @@ static void tegra_sor_hdmi_scdc_work(struct work_struct
*work)
static void tegra_sor_hdmi_scdc_start(struct tegra_sor *sor)
{
struct drm_scdc *scdc = >output.connector.display_info.hdmi.scdc;
- struct drm_display_mode *mode;
+ const struct drm_display_mode *mode;
mode = >output.encoder.crtc->state->adjusted_mode;
- if (mode->clock >= DRM_HDMI_14_MAX_TMDS_CLK_KHZ && scdc->supported) {
+ if (drm_mode_hdmi_requires_scrambling(mode) && scdc->supported) {
schedule_delayed_work(>scdc, msecs_to_jiffies(5000));
tegra_sor_hdmi_scdc_enable(sor);
sor->scdc_enabled = true;
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index fc7247cc1022..fa76ae2c94e4 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -99,11 +99,6 @@
#define HDMI_14_MAX_TMDS_CLK (DRM_HDMI_14_MAX_TMDS_CLK_KHZ * 1000)
-static bool vc4_hdmi_mode_needs_scrambling(const struct drm_display_mode *mode)
-{
- return mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ;
-}
-
static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
{
struct drm_info_node *node = (struct drm_info_node *)m->private;
@@ -260,10 +255,10 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
if (vc4_hdmi->disable_4kp60) {
struct drm_device *drm = connector->dev;
- struct drm_display_mode *mode;
+ const struct drm_display_mode *mode;
list_for_each_entry(mode, >probed_modes, head) {
- if (vc4_hdmi_mode_needs_scrambling(mode)) {
+ if (drm_mode_hdmi_requires_scrambling(mode)) {
drm_warn_once(drm, "The core clock cannot reach
frequencies high enough to support 4k @ 60Hz.");
drm_warn_once(drm, "Please change your
config.txt file to add hdmi_enable_4kp60.");
}
@@ -574,7 +569,7 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder
*encoder)
}
static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder,
-struct drm_display_mode *mode)
+const struct drm_display_mode *mode)
{
struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
@@ -597,7 +592,7 @@ static bool vc4_hdmi_supports_scrambling(struct drm_encoder
*encoder,
static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder)
{
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
- struct drm_display_mode *mode = _hdmi->saved_adjusted_mode;
+ const struct drm_display_mode *mode = _hdmi->saved_adjusted_mode;
unsigned long flags;
lockdep_assert_held(_hdmi->mutex);
@@ -605,7 +600,7 @@ static void vc4_hdmi_enable_scrambling(struct drm_encoder
*encoder)
if (!vc4_hdmi_supports_scrambling(encoder, mode))
return;
- if (!vc4_hdmi_mode_needs_scrambling(mode))
+ if (!drm_mode_hdmi_requires_scrambling(mode))
return;
drm_scdc_set_high_tmds_clock_ratio(vc4_hdmi->ddc, true);
@@ -1283,7 +1278,8 @@ static int vc4_hdmi_encoder_atomic_check(struct
drm_encoder *encoder,
if (pixel_rate > vc4_hdmi->variant->max_pixel_clock)
return -EINVAL;
- if (vc4_hdmi->disable_4kp60 && (pixel_rate > HDMI_14_MAX_TMDS_CLK))
+ if (vc4_hdmi->disable_4kp60 &&
+ drm_mode_hdmi_requires_scrambling(mode))
return -EINVAL;
vc4_state->pixel_rate = pixel_rate;
@@ -1305,7 +1301,8 @@ vc4_hdmi_encoder_mode_valid(struct drm_encoder *encoder,
if ((mode->clock * 1000) > vc4_hdmi->variant->max_pixel_clock)
return MODE_CLOCK_HIGH;
- if (vc4_hdmi->disable_4kp60 && vc4_hdmi_mode_needs_scrambling(mode))
+ if (vc4_hdmi->disable_4kp60 &&
+ drm_mode_hdmi_requires_scrambling(mode))
return MODE_CLOCK_HIGH;
return MODE_OK;
diff --git
[PATCH 00/13] drm: Add generic helpers for HDMI scrambling
Hi, This is a follow-up of the work to support the interactions between the hotplug and the scrambling support for vc4: https://lore.kernel.org/dri-devel/20210507150515.257424-11-max...@cerno.tech/ https://lore.kernel.org/dri-devel/20211025152903.1088803-10-max...@cerno.tech/ Ville feedback was that the same discussion happened some time ago for i915, and resulted in a function to do an full disable/enable cycle on reconnection to avoid breaking the HDMI 2.0 spec. This series improves the current scrambling support by adding generic helpers for usual scrambling-related operations, and builds upon them to provide a generic alternative to intel_hdmi_reset_link. Let me know what you think, Maxime Maxime Ripard (13): drm/connector: Add define for HDMI 1.4 Maximum Pixel Rate drm/connector: Add helper to check if a mode requires scrambling drm/atomic: Add HDMI scrambler state helper drm/atomic: Add HDMI reset link helper drm/scdc: Document hotplug gotchas drm/vc4: hdmi: Remove unused argument in vc4_hdmi_supports_scrambling drm/vc4: hdmi: Remove mutex in detect drm/vc4: hdmi: Remove HDMI flag from encoder drm/vc4: hdmi: Simplify the hotplug handling drm/vc4: hdmi: Simplify the connector state retrieval drm/vc4: hdmi: Switch to detect_ctx drm/vc4: hdmi: Leverage new SCDC atomic_check drm/vc4: hdmi: Reset link on hotplug drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 +- drivers/gpu/drm/drm_atomic_helper.c| 109 ++ drivers/gpu/drm/drm_atomic_state_helper.c | 58 ++ drivers/gpu/drm/drm_edid.c | 2 +- drivers/gpu/drm/drm_scdc_helper.c | 13 ++ drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +- drivers/gpu/drm/meson/meson_dw_hdmi.c | 4 +- drivers/gpu/drm/radeon/radeon_encoders.c | 2 +- drivers/gpu/drm/sti/sti_hdmi_tx3g4c28phy.c | 2 +- drivers/gpu/drm/tegra/sor.c| 10 +- drivers/gpu/drm/vc4/vc4_hdmi.c | 232 + drivers/gpu/drm/vc4/vc4_hdmi.h | 17 +- include/drm/drm_atomic_helper.h| 3 + include/drm/drm_atomic_state_helper.h | 3 + include/drm/drm_connector.h| 27 +++ include/drm/drm_modes.h| 15 ++ 16 files changed, 342 insertions(+), 161 deletions(-) -- 2.32.0
Re: [PATCH 02/29] drm/i915/gvt: integrate into the main Makefile
On Tue, 02 Nov 2021, Christoph Hellwig wrote: > Remove the separately included Makefile and just use the relative > reference from the main i915 Makefile as for source files in other > subdirectories. > > Signed-off-by: Christoph Hellwig Acked-by: Jani Nikula > --- > drivers/gpu/drm/i915/Makefile | 29 - > drivers/gpu/drm/i915/gvt/Makefile | 9 - > drivers/gpu/drm/i915/gvt/trace.h | 2 +- > 3 files changed, 25 insertions(+), 15 deletions(-) > delete mode 100644 drivers/gpu/drm/i915/gvt/Makefile > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index 335ba9f43d8f7..63523032eea26 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -295,11 +295,30 @@ i915-$(CONFIG_DRM_I915_SELFTEST) += \ > > # virtual gpu code > i915-y += i915_vgpu.o > - > -ifeq ($(CONFIG_DRM_I915_GVT),y) > -i915-y += intel_gvt.o > -include $(src)/gvt/Makefile > -endif > +i915-$(CONFIG_DRM_I915_GVT) += \ > + intel_gvt.o \ > + gvt/gvt.o \ > + gvt/aperture_gm.o \ > + gvt/handlers.o \ > + gvt/vgpu.o \ > + gvt/trace_points.o \ > + gvt/firmware.o \ > + gvt/interrupt.o \ > + gvt/gtt.o \ > + gvt/cfg_space.o \ > + gvt/opregion.o \ > + gvt/mmio.o \ > + gvt/display.o \ > + gvt/edid.o \ > + gvt/execlist.o \ > + gvt/scheduler.o \ > + gvt/sched_policy.o \ > + gvt/mmio_context.o \ > + gvt/cmd_parser.o \ > + gvt/debugfs.o \ > + gvt/fb_decoder.o \ > + gvt/dmabuf.o \ > + gvt/page_track.o > > obj-$(CONFIG_DRM_I915) += i915.o > obj-$(CONFIG_DRM_I915_GVT_KVMGT) += gvt/kvmgt.o > diff --git a/drivers/gpu/drm/i915/gvt/Makefile > b/drivers/gpu/drm/i915/gvt/Makefile > deleted file mode 100644 > index ea8324abc784a..0 > --- a/drivers/gpu/drm/i915/gvt/Makefile > +++ /dev/null > @@ -1,9 +0,0 @@ > -# SPDX-License-Identifier: GPL-2.0 > -GVT_DIR := gvt > -GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o > firmware.o \ > - interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \ > - execlist.o scheduler.o sched_policy.o mmio_context.o cmd_parser.o > debugfs.o \ > - fb_decoder.o dmabuf.o page_track.o > - > -ccflags-y+= -I $(srctree)/$(src) -I > $(srctree)/$(src)/$(GVT_DIR)/ > -i915-y += $(addprefix $(GVT_DIR)/, > $(GVT_SOURCE)) > diff --git a/drivers/gpu/drm/i915/gvt/trace.h > b/drivers/gpu/drm/i915/gvt/trace.h > index 6d787750d279f..348f57f8301db 100644 > --- a/drivers/gpu/drm/i915/gvt/trace.h > +++ b/drivers/gpu/drm/i915/gvt/trace.h > @@ -379,5 +379,5 @@ TRACE_EVENT(render_mmio, > #undef TRACE_INCLUDE_PATH > #define TRACE_INCLUDE_PATH . > #undef TRACE_INCLUDE_FILE > -#define TRACE_INCLUDE_FILE trace > +#define TRACE_INCLUDE_FILE gvt/trace > #include -- Jani Nikula, Intel Open Source Graphics Center
[PATCH v3 3/3] arm64: dts: qcom: sc7280: add edp display dt nodes
Add edp controller and phy DT nodes for sc7280.
Signed-off-by: Krishna Manikandan
Reviewed-by: Stephen Boyd
Signed-off-by: Sankeerth Billakanti
---
Changes in v3:
- Add one clock cell per line (Stephen Boyd)
- Unit address should match first reg property (Stephen Boyd)
- Remove new line (Stephen Boyd)
- Add the dsi_phy clocks in dispcc (Kuogee Hsieh)
Changes in v2:
- Move regulator definitions to board file (Matthias Kaehlcke)
- Move the gpio definitions to board file (Matthias Kaehlcke)
- Move the pinconf to board file (Matthias Kaehlcke)
- Move status property (Stephen Boyd)
- Drop flags from interrupts (Stephen Boyd)
- Add clock names one per line for readability (Stephen Boyd)
- Rename edp-opp-table (Stephen Boyd)
arch/arm64/boot/dts/qcom/sc7280.dtsi | 107 ++-
1 file changed, 105 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 12c4d32..5ad500e 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2695,8 +2695,8 @@
<_phy 1>,
<0>,
<0>,
-<0>,
-<0>;
+<_phy 0>,
+<_phy 1>;
clock-names = "bi_tcxo",
"gcc_disp_gpll0_clk",
"dsi0_phy_pll_out_byteclk",
@@ -2784,6 +2784,13 @@
remote-endpoint =
<_in>;
};
};
+
+ port@1 {
+ reg = <1>;
+ dpu_intf5_out: endpoint {
+ remote-endpoint =
<_in>;
+ };
+ };
};
mdp_opp_table: opp-table {
@@ -2899,6 +2906,102 @@
status = "disabled";
};
+
+ msm_edp: edp@aea {
+ compatible = "qcom,sc7280-edp";
+
+ reg = <0 0xaea 0 0x200>,
+ <0 0xaea0200 0 0x200>,
+ <0 0xaea0400 0 0xc00>,
+ <0 0xaea1000 0 0x400>;
+
+ interrupt-parent = <>;
+ interrupts = <14>;
+
+ clocks = < RPMH_CXO_CLK>,
+< GCC_EDP_CLKREF_EN>,
+< DISP_CC_MDSS_AHB_CLK>,
+< DISP_CC_MDSS_EDP_AUX_CLK>,
+< DISP_CC_MDSS_EDP_LINK_CLK>,
+<
DISP_CC_MDSS_EDP_LINK_INTF_CLK>,
+< DISP_CC_MDSS_EDP_PIXEL_CLK>;
+ clock-names = "core_xo",
+ "core_ref",
+ "core_iface",
+ "core_aux",
+ "ctrl_link",
+ "ctrl_link_iface",
+ "stream_pixel";
+ #clock-cells = <1>;
+ assigned-clocks = <
DISP_CC_MDSS_EDP_LINK_CLK_SRC>,
+ <
DISP_CC_MDSS_EDP_PIXEL_CLK_SRC>;
+ assigned-clock-parents = <_phy 0>,
<_phy 1>;
+
+ phys = <_phy>;
+ phy-names = "dp";
+
+ operating-points-v2 = <_opp_table>;
+ power-domains = < SC7280_CX>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ edp_in: endpoint {
+ remote-endpoint =
<_intf5_out>;
+ };
+ };
+ };
+
+
[PATCH v3 2/3] arm64: dts: qcom: sc7280: Add DSI display nodes
From: Krishna Manikandan
Add DSI controller and PHY nodes for sc7280.
Signed-off-by: Rajeev Nandan
Signed-off-by: Krishna Manikandan
Reviewed-by: Matthias Kaehlcke
Reviewed-by: Stephen Boyd
Signed-off-by: Sankeerth Billakanti
---
Changes in v3:
- Add the dsi_phy clocks (Kuogee Hsieh)
- One clock cell per line (Stephen Boyd)
Changes in v2:
- Drop flags from interrupts (Stephen Boyd)
- Rename dsi-opp-table (Stephen Boyd)
- Rename dsi phy node (Stephen Boyd)
arch/arm64/boot/dts/qcom/sc7280.dtsi | 111 ++-
1 file changed, 109 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index a4536b6..12c4d32 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2691,8 +2691,14 @@
reg = <0 0xaf0 0 0x2>;
clocks = < RPMH_CXO_CLK>,
< GCC_DISP_GPLL0_CLK_SRC>,
-<0>, <0>, <0>, <0>, <0>, <0>;
- clock-names = "bi_tcxo", "gcc_disp_gpll0_clk",
+<_phy 0>,
+<_phy 1>,
+<0>,
+<0>,
+<0>,
+<0>;
+ clock-names = "bi_tcxo",
+ "gcc_disp_gpll0_clk",
"dsi0_phy_pll_out_byteclk",
"dsi0_phy_pll_out_dsiclk",
"dp_phy_pll_link_clk",
@@ -2768,6 +2774,18 @@
status = "disabled";
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ dpu_intf1_out: endpoint {
+ remote-endpoint =
<_in>;
+ };
+ };
+ };
+
mdp_opp_table: opp-table {
compatible = "operating-points-v2";
@@ -2792,6 +2810,95 @@
};
};
};
+
+ dsi0: dsi@ae94000 {
+ compatible = "qcom,mdss-dsi-ctrl";
+ reg = <0 0x0ae94000 0 0x400>;
+ reg-names = "dsi_ctrl";
+
+ interrupt-parent = <>;
+ interrupts = <4>;
+
+ clocks = < DISP_CC_MDSS_BYTE0_CLK>,
+< DISP_CC_MDSS_BYTE0_INTF_CLK>,
+< DISP_CC_MDSS_PCLK0_CLK>,
+< DISP_CC_MDSS_ESC0_CLK>,
+< DISP_CC_MDSS_AHB_CLK>,
+< GCC_DISP_HF_AXI_CLK>;
+ clock-names = "byte",
+ "byte_intf",
+ "pixel",
+ "core",
+ "iface",
+ "bus";
+
+ operating-points-v2 = <_opp_table>;
+ power-domains = < SC7280_CX>;
+
+ phys = <_phy>;
+ phy-names = "dsi";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ dsi0_in: endpoint {
+ remote-endpoint =
<_intf1_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ dsi0_out: endpoint {
+ };
+ };
+ };
+
+ dsi_opp_table: opp-table {
+
[PATCH v3 1/3] arm64: dts: qcom: sc7280: add display dt nodes
From: Krishna Manikandan
Add mdss and mdp DT nodes for sc7280.
Signed-off-by: Krishna Manikandan
Reported-by: kernel test robot
Reviewed-by: Stephen Boyd
Reported-by: kernel test robot
Signed-off-by: Sankeerth Billakanti
---
Changes in v3:
None
Changes in v2:
- Rename display dt nodes (Stephen Boyd)
- Add clock names one per line for readability (Stephen Boyd)
arch/arm64/boot/dts/qcom/sc7280.dtsi | 90
1 file changed, 90 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 365a2e0..a4536b6 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2704,6 +2704,96 @@
#power-domain-cells = <1>;
};
+ mdss: display-subsystem@ae0 {
+ compatible = "qcom,sc7280-mdss";
+ reg = <0 0x0ae0 0 0x1000>;
+ reg-names = "mdss";
+
+ power-domains = < DISP_CC_MDSS_CORE_GDSC>;
+
+ clocks = < GCC_DISP_AHB_CLK>,
+< DISP_CC_MDSS_AHB_CLK>,
+ < DISP_CC_MDSS_MDP_CLK>;
+ clock-names = "iface",
+ "ahb",
+ "core";
+
+ assigned-clocks = < DISP_CC_MDSS_MDP_CLK>;
+ assigned-clock-rates = <3>;
+
+ interrupts = ;
+ interrupt-controller;
+ #interrupt-cells = <1>;
+
+ interconnects = <_noc MASTER_MDP0 0 _virt
SLAVE_EBI1 0>;
+ interconnect-names = "mdp0-mem";
+
+ iommus = <_smmu 0x900 0x402>;
+
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ status = "disabled";
+
+ mdp: display-controller@ae01000 {
+ compatible = "qcom,sc7280-dpu";
+ reg = <0 0x0ae01000 0 0x8f030>,
+ <0 0x0aeb 0 0x2008>;
+ reg-names = "mdp", "vbif";
+
+ clocks = < GCC_DISP_HF_AXI_CLK>,
+ < GCC_DISP_SF_AXI_CLK>,
+ < DISP_CC_MDSS_AHB_CLK>,
+ < DISP_CC_MDSS_MDP_LUT_CLK>,
+ < DISP_CC_MDSS_MDP_CLK>,
+ < DISP_CC_MDSS_VSYNC_CLK>;
+ clock-names = "bus",
+ "nrt_bus",
+ "iface",
+ "lut",
+ "core",
+ "vsync";
+ assigned-clocks = <
DISP_CC_MDSS_MDP_CLK>,
+ <
DISP_CC_MDSS_VSYNC_CLK>,
+ < DISP_CC_MDSS_AHB_CLK>;
+ assigned-clock-rates = <3>,
+ <1920>,
+ <1920>;
+ operating-points-v2 = <_opp_table>;
+ power-domains = < SC7280_CX>;
+
+ interrupt-parent = <>;
+ interrupts = <0>;
+
+ status = "disabled";
+
+ mdp_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-2 {
+ opp-hz = /bits/ 64 <2>;
+ required-opps =
<_opp_low_svs>;
+ };
+
+ opp-3 {
+ opp-hz = /bits/ 64 <3>;
+ required-opps =
<_opp_svs>;
+ };
+
+ opp-38000 {
+ opp-hz = /bits/ 64 <38000>;
+ required-opps =
<_opp_svs_l1>;
+ };
+
+ opp-50667 {
+ opp-hz = /bits/ 64 <50667>;
+ required-opps =
<_opp_nom>;
+ };
+ };
+
Re: [PATCH v2 2/2] drm/i915/ttm: Failsafe migration blits
Thanks for reviewing Matt,
On 11/2/21 14:55, Matthew Auld wrote:
On 01/11/2021 18:38, Thomas Hellström wrote:
If the initial fill blit or copy blit of an object fails, the old
content of the data might be exposed and read as soon as either CPU- or
GPU PTEs are set up to point at the pages.
Intercept the blit fence with an async callback that checks the
blit fence for errors and if there are errors performs an async cpu blit
instead. If there is a failure to allocate the async dma_fence_work,
allocate it on the stack and sync wait for the blit to complete.
Add selftests that simulate gpu blit failures and failure to allocate
the async dma_fence_work.
A previous version of this pach used dma_fence_work, now that's
opencoded which adds more code but might lower the latency
somewhat in the common non-error case.
Signed-off-by: Thomas Hellström
---
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 322 +++---
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h | 5 +
.../drm/i915/gem/selftests/i915_gem_migrate.c | 24 +-
3 files changed, 295 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
index 0ed6b7f2b95f..2e3328e2b48c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -18,6 +18,29 @@
#include "gt/intel_gt.h"
#include "gt/intel_migrate.h"
+/**
+ * DOC: Selftest failure modes for failsafe migration:
+ *
+ * For fail_gpu_migration, the gpu blit scheduled is always a clear
blit
+ * rather than a copy blit, and then we force the failure paths as if
+ * the blit fence returned an error.
+ *
+ * For fail_work_allocation we fail the kmalloc of the async worker, we
+ * sync the gpu blit. If it then fails, or fail_gpu_migration is set to
+ * true, then a memcpy operation is performed sync.
+ */
+I915_SELFTEST_DECLARE(static bool fail_gpu_migration;)
+I915_SELFTEST_DECLARE(static bool fail_work_allocation;)
Might as well move these under the CONFIG_SELFTEST below, and then
drop the DECLARE stuff?
+
+#ifdef CONFIG_DRM_I915_SELFTEST
+void i915_ttm_migrate_set_failure_modes(bool gpu_migration,
+ bool work_allocation)
+{
+ fail_gpu_migration = gpu_migration;
+ fail_work_allocation = work_allocation;
+}
+#endif
+
static enum i915_cache_level
i915_ttm_cache_level(struct drm_i915_private *i915, struct
ttm_resource *res,
struct ttm_tt *ttm)
@@ -129,11 +152,11 @@ int i915_ttm_move_notify(struct
ttm_buffer_object *bo)
return 0;
}
-static int i915_ttm_accel_move(struct ttm_buffer_object *bo,
- bool clear,
- struct ttm_resource *dst_mem,
- struct ttm_tt *dst_ttm,
- struct sg_table *dst_st)
+static struct dma_fence *i915_ttm_accel_move(struct
ttm_buffer_object *bo,
+ bool clear,
+ struct ttm_resource *dst_mem,
+ struct ttm_tt *dst_ttm,
+ struct sg_table *dst_st)
{
struct drm_i915_private *i915 = container_of(bo->bdev,
typeof(*i915),
bdev);
@@ -144,30 +167,29 @@ static int i915_ttm_accel_move(struct
ttm_buffer_object *bo,
int ret;
if (!i915->gt.migrate.context || intel_gt_is_wedged(>gt))
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
+
+ /* With fail_gpu_migration, we always perform a GPU clear. */
+ if (I915_SELFTEST_ONLY(fail_gpu_migration))
+ clear = true;
dst_level = i915_ttm_cache_level(i915, dst_mem, dst_ttm);
if (clear) {
- if (bo->type == ttm_bo_type_kernel)
- return -EINVAL;
+ if (bo->type == ttm_bo_type_kernel &&
+ !I915_SELFTEST_ONLY(fail_gpu_migration))
+ return ERR_PTR(-EINVAL);
intel_engine_pm_get(i915->gt.migrate.context->engine);
ret = intel_context_migrate_clear(i915->gt.migrate.context,
NULL,
dst_st->sgl, dst_level,
i915_ttm_gtt_binds_lmem(dst_mem),
0, );
-
- if (!ret && rq) {
- i915_request_wait(rq, 0, MAX_SCHEDULE_TIMEOUT);
- i915_request_put(rq);
- }
- intel_engine_pm_put(i915->gt.migrate.context->engine);
} else {
struct i915_refct_sgt *src_rsgt =
i915_ttm_resource_get_st(obj, bo->resource);
if (IS_ERR(src_rsgt))
- return PTR_ERR(src_rsgt);
+ return ERR_CAST(src_rsgt);
src_level = i915_ttm_cache_level(i915, bo->resource,
src_ttm);
intel_engine_pm_get(i915->gt.migrate.context->engine);
@@ -178,15 +200,183 @@ static int i915_ttm_accel_move(struct
ttm_buffer_object *bo,
dst_st->sgl, dst_level,
i915_ttm_gtt_binds_lmem(dst_mem),
);
+
i915_refct_sgt_put(src_rsgt);
-
Re: [PATCH v2 06/13] drm/exynos: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
On Wed, Oct 27, 2021 at 07:28:45AM +0900, Inki Dae wrote: > Hi, > > 21. 10. 17. 오전 3:42에 Claudio Suarez 이(가) 쓴 글: > > Once EDID is parsed, the monitor HDMI support information is available > > through drm_display_info.is_hdmi. Retriving the same information with > > drm_detect_hdmi_monitor() is less efficient. Change to > > drm_display_info.is_hdmi > > > > Signed-off-by: Claudio Suarez > > --- > > drivers/gpu/drm/exynos/exynos_hdmi.c | 6 -- > > 1 file changed, 4 insertions(+), 2 deletions( > > > > diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c > > b/drivers/gpu/drm/exynos/exynos_hdmi.c > > index 7655142a4651..a563d6386abe 100644 > > --- a/drivers/gpu/drm/exynos/exynos_hdmi.c > > +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c > > @@ -893,12 +893,14 @@ static int hdmi_get_modes(struct drm_connector > > *connector) > > if (!edid) > > return -ENODEV; > > > > - hdata->dvi_mode = !drm_detect_hdmi_monitor(edid); > > + /* This updates connector->display_info */ > > + drm_connector_update_edid_property(connector, edid); > > + > > + hdata->dvi_mode = !connector->display_info.is_hdmi; > > Thanks for correcting this. Yeah, we should use drm_display_info.is_hdmi > parsed from EDID. > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpu/drm/drm_edid.c?h=v5.14.14#n4725 > > Signed-off-by: Inki Dae Thank you, Inki. Best regards Claudio Suarez
Re: [Intel-gfx] [PATCH v3 05/10] drm/i915: Prepare for multiple gts
On 02/11/2021 11:26, Andi Shyti wrote: Hi Tvrtko, [...] static int intel_gt_tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t phys_addr) we don't actually need 'id', it's gt->info.id. It's introduced in patch 3 with the value '0' but it's not needed. I have a suspicion code got munged up over endless rebases and refactors. This patch is the one which introduces the id member to gt->info. But it is not setting it, even though I suspect the intent was for intel_gt_tile_setup to do that. Instead gt->info.id is only set to a valid value in last patch of this series inside intel_gt_probe_all: + gt->i915 = i915; + gt->name = gtdef->name; + gt->type = gtdef->type; + gt->info.engine_mask = gtdef->engine_mask; + gt->info.id = i; + + drm_dbg(>drm, "Setting up %s %u\n", gt->name, gt->info.id); + ret = intel_gt_tile_setup(gt, i, phys_addr + gtdef->mapping_base); And intel_gt_tile_setup then calls __intel_gt_init_early which assigns gt->i915 yet again. So I'd say there is probably space to bring this all into a more streamlined flow, even more than what you suggest below. yes, I noticed them! Patch 3, 5 and 10 are very much connected with each other: 3 prepares for one tile, 5 prepares for multitile and 10 does the multitile. While in between other patches are doing other things. On top of some cleanups we could also rearrange the patches with some squashing and reordering to have them a bit more linear and also easier to review. Yes. Maybe make intel_gt_tile_setup accept more arguments so it can be truly used to setup a gt? intel_gt_tile_setup(gt, id, name, type, engine_mask) The usual thing where patch which adds something extends the prototype to include more stuff. If that applies here. I know it is originally my patch but I don't have the time to rework it, much less the whole series, so usual dispensation to take over authorship if changes are large applies. Regards, Tvrtko
Re: [PATCH 02/29] drm/i915/gvt: integrate into the main Makefile
On Tue, Nov 02, 2021 at 08:05:34AM +0100, Christoph Hellwig wrote: > Remove the separately included Makefile and just use the relative > reference from the main i915 Makefile as for source files in other > subdirectories. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/Makefile | 29 - > drivers/gpu/drm/i915/gvt/Makefile | 9 - > drivers/gpu/drm/i915/gvt/trace.h | 2 +- > 3 files changed, 25 insertions(+), 15 deletions(-) > delete mode 100644 drivers/gpu/drm/i915/gvt/Makefile Reviewed-by: Jason Gunthorpe > - > -ifeq ($(CONFIG_DRM_I915_GVT),y) > -i915-y += intel_gvt.o > -include $(src)/gvt/Makefile > -endif > +i915-$(CONFIG_DRM_I915_GVT) += \ > + intel_gvt.o \ > + gvt/gvt.o \ > + gvt/aperture_gm.o \ > + gvt/handlers.o \ > + gvt/vgpu.o \ > + gvt/trace_points.o \ > + gvt/firmware.o \ > + gvt/interrupt.o \ > + gvt/gtt.o \ > + gvt/cfg_space.o \ > + gvt/opregion.o \ > + gvt/mmio.o \ > + gvt/display.o \ > + gvt/edid.o \ > + gvt/execlist.o \ > + gvt/scheduler.o \ > + gvt/sched_policy.o \ > + gvt/mmio_context.o \ > + gvt/cmd_parser.o \ > + gvt/debugfs.o \ > + gvt/fb_decoder.o \ > + gvt/dmabuf.o \ > + gvt/page_track.o nit, nicer to sort makefile lists Jason
Re: [PATCH v2 2/2] drm/i915/ttm: Failsafe migration blits
On 01/11/2021 18:38, Thomas Hellström wrote:
If the initial fill blit or copy blit of an object fails, the old
content of the data might be exposed and read as soon as either CPU- or
GPU PTEs are set up to point at the pages.
Intercept the blit fence with an async callback that checks the
blit fence for errors and if there are errors performs an async cpu blit
instead. If there is a failure to allocate the async dma_fence_work,
allocate it on the stack and sync wait for the blit to complete.
Add selftests that simulate gpu blit failures and failure to allocate
the async dma_fence_work.
A previous version of this pach used dma_fence_work, now that's
opencoded which adds more code but might lower the latency
somewhat in the common non-error case.
Signed-off-by: Thomas Hellström
---
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 322 +++---
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h | 5 +
.../drm/i915/gem/selftests/i915_gem_migrate.c | 24 +-
3 files changed, 295 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
index 0ed6b7f2b95f..2e3328e2b48c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -18,6 +18,29 @@
#include "gt/intel_gt.h"
#include "gt/intel_migrate.h"
+/**
+ * DOC: Selftest failure modes for failsafe migration:
+ *
+ * For fail_gpu_migration, the gpu blit scheduled is always a clear blit
+ * rather than a copy blit, and then we force the failure paths as if
+ * the blit fence returned an error.
+ *
+ * For fail_work_allocation we fail the kmalloc of the async worker, we
+ * sync the gpu blit. If it then fails, or fail_gpu_migration is set to
+ * true, then a memcpy operation is performed sync.
+ */
+I915_SELFTEST_DECLARE(static bool fail_gpu_migration;)
+I915_SELFTEST_DECLARE(static bool fail_work_allocation;)
Might as well move these under the CONFIG_SELFTEST below, and then drop
the DECLARE stuff?
+
+#ifdef CONFIG_DRM_I915_SELFTEST
+void i915_ttm_migrate_set_failure_modes(bool gpu_migration,
+ bool work_allocation)
+{
+ fail_gpu_migration = gpu_migration;
+ fail_work_allocation = work_allocation;
+}
+#endif
+
static enum i915_cache_level
i915_ttm_cache_level(struct drm_i915_private *i915, struct ttm_resource *res,
struct ttm_tt *ttm)
@@ -129,11 +152,11 @@ int i915_ttm_move_notify(struct ttm_buffer_object *bo)
return 0;
}
-static int i915_ttm_accel_move(struct ttm_buffer_object *bo,
- bool clear,
- struct ttm_resource *dst_mem,
- struct ttm_tt *dst_ttm,
- struct sg_table *dst_st)
+static struct dma_fence *i915_ttm_accel_move(struct ttm_buffer_object *bo,
+bool clear,
+struct ttm_resource *dst_mem,
+struct ttm_tt *dst_ttm,
+struct sg_table *dst_st)
{
struct drm_i915_private *i915 = container_of(bo->bdev, typeof(*i915),
bdev);
@@ -144,30 +167,29 @@ static int i915_ttm_accel_move(struct ttm_buffer_object
*bo,
int ret;
if (!i915->gt.migrate.context || intel_gt_is_wedged(>gt))
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
+
+ /* With fail_gpu_migration, we always perform a GPU clear. */
+ if (I915_SELFTEST_ONLY(fail_gpu_migration))
+ clear = true;
dst_level = i915_ttm_cache_level(i915, dst_mem, dst_ttm);
if (clear) {
- if (bo->type == ttm_bo_type_kernel)
- return -EINVAL;
+ if (bo->type == ttm_bo_type_kernel &&
+ !I915_SELFTEST_ONLY(fail_gpu_migration))
+ return ERR_PTR(-EINVAL);
intel_engine_pm_get(i915->gt.migrate.context->engine);
ret = intel_context_migrate_clear(i915->gt.migrate.context,
NULL,
dst_st->sgl, dst_level,
i915_ttm_gtt_binds_lmem(dst_mem),
0, );
-
- if (!ret && rq) {
- i915_request_wait(rq, 0, MAX_SCHEDULE_TIMEOUT);
- i915_request_put(rq);
- }
- intel_engine_pm_put(i915->gt.migrate.context->engine);
} else {
struct i915_refct_sgt *src_rsgt =
i915_ttm_resource_get_st(obj, bo->resource);
if (IS_ERR(src_rsgt))
- return PTR_ERR(src_rsgt);
+ return ERR_CAST(src_rsgt);
src_level = i915_ttm_cache_level(i915, bo->resource, src_ttm);
Re: [PATCH v8, 15/17] dt-bindings: media: mtk-vcodec: Adds decoder dt-bindings for mt8192
On Fri, Oct 29, 2021 at 11:55:25AM +0800, Yunfei Dong wrote: > Adds decoder dt-bindings for mt8192. > > Signed-off-by: Yunfei Dong > --- > v8: fix yaml file check fail > --- > .../media/mediatek,vcodec-comp-decoder.yaml | 273 ++ > 1 file changed, 273 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/media/mediatek,vcodec-comp-decoder.yaml > > diff --git > a/Documentation/devicetree/bindings/media/mediatek,vcodec-comp-decoder.yaml > b/Documentation/devicetree/bindings/media/mediatek,vcodec-comp-decoder.yaml > new file mode 100644 > index ..40a076756439 > --- /dev/null > +++ > b/Documentation/devicetree/bindings/media/mediatek,vcodec-comp-decoder.yaml > @@ -0,0 +1,273 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > + > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/media/mediatek,vcodec-comp-decoder.yaml#; > +$schema: "http://devicetree.org/meta-schemas/core.yaml#; > + > +title: Mediatek Video Decode Accelerator With Multi Hardware > + > +maintainers: > + - Yunfei Dong > + > +description: | > + Mediatek Video Decode is the video decode hardware present in Mediatek > + SoCs which supports high resolution decoding functionalities. Required > + master and component node. > + > + About the Decoder Hardware Block Diagram, please check below: > + > ++-++ > +| || > +| input -> lat HW -> lat buffer --|--> lat buffer -> core HW -> output | > +||| | || | > ++||---+-||-+ > + || lat thread | core thread|| > + > -||-|| > + || || > > + \/\/ > + +--+ > + |enable/disable| > + | clk powerirqiommu port | > + | (lat/lat soc/core0/core1)| > + +--+ > + > + As above, mean in master device, mean in component > device. > + The information of each hardware will be stored in each component device. > There > + are two workqueue in master device: lat and core. Enable/disable the lat > clk/power/irq > + when lat hardware need to work through hardware index, core is the same. > + > + Normally the smi common may not the same for each hardware, can't combine > all > + hardware in one node, or leading to iommu fault when access dram data. > + > +properties: > + compatible: > +const: mediatek,mt8192-vcodec-dec > + > + reg: > +maxItems: 1 > + > + iommus: > +minItems: 1 > +maxItems: 32 > +description: | > + List of the hardware port in respective IOMMU block for current Socs. > + Refer to bindings/iommu/mediatek,iommu.yaml. > + > + mediatek,scp: > +$ref: /schemas/types.yaml#/definitions/phandle > +maxItems: 1 > +description: | > + The node of system control processor (SCP), using > + the remoteproc & rpmsg framework. > + $ref: /schemas/remoteproc/mtk,scp.yaml > + > + dma-ranges: > +maxItems: 1 > +description: | > + Describes the physical address space of IOMMU maps to memory. > + > + "#address-cells": > +const: 1 > + > + "#size-cells": > +const: 1 > + > + ranges: true > + > +# Required child node: > +patternProperties: > + vcodec-lat: > +type: object > + > +properties: > + compatible: > +const: mediatek,mtk-vcodec-lat > + > + reg: > +maxItems: 1 > + > + reg-names: > +maxItems: 1 You have to document what the names are. But 'misc' is isn't really specific and you don't need -names when there is only 1, so I'd just drop it. > + > + interrupts: > +maxItems: 1 > + > + iommus: > +minItems: 1 > +maxItems: 32 > +description: | > + List of the hardware port in respective IOMMU block for current > Socs. > + Refer to bindings/iommu/mediatek,iommu.yaml. > + > + clocks: > +maxItems: 5 > + > + clock-names: > +items: > + - const: vdec-sel > + - const: vdec-soc-vdec > + - const: vdec-soc-lat > + - const: vdec-vdec > + - const: vdec-top 'vdec-' is redundant. Names are local to the node. > + > + assigned-clocks: > +maxItems: 1 > + > + assigned-clock-parents: > +maxItems: 1 > + > + power-domains: > +maxItems: 1 > + > +required: > + - compatible > + - reg > + - reg-names > + -
