From: Hsiao Chien Sung <[email protected]>
Set DRM mode configs limitation according to the hardware capabilities
and pass the IGT checks as below:
- The test "graphics.IgtKms.kms_plane" requires a frame buffer with
width of 4512 pixels (> 4096).
- The test "graphics.IgtKms.kms_cursor_crc" checks if the cursor size is
defined, and run the test with cursor size from 1x1 to 512x512.
Please notice that the test conditions may change as IGT is updated.
Signed-off-by: Hsiao Chien Sung <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 22 ++++++++++++++++++++++
drivers/gpu/drm/mediatek/mtk_drm_drv.h | 4 ++++
2 files changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 8e047043202b4..c9cad3a827376 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -294,6 +294,9 @@ static const struct mtk_mmsys_driver_data mt8188_vdosys0_driver_data = {
.conn_routes = mt8188_mtk_ddp_main_routes,
.num_conn_routes = ARRAY_SIZE(mt8188_mtk_ddp_main_routes),
.mmsys_dev_num = 2,
+ .max_width = 8191,
+ .min_width = 1,
+ .min_height = 1,
};
static const struct mtk_mmsys_driver_data mt8192_mmsys_driver_data = {
@@ -308,6 +311,9 @@ static const struct mtk_mmsys_driver_data mt8195_vdosys0_driver_data = {
.main_path = mt8195_mtk_ddp_main,
.main_len = ARRAY_SIZE(mt8195_mtk_ddp_main),
.mmsys_dev_num = 2,
+ .max_width = 8191,
+ .min_width = 1,
+ .min_height = 1,
};
static const struct mtk_mmsys_driver_data mt8195_vdosys1_driver_data = {
@@ -315,6 +321,9 @@ static const struct mtk_mmsys_driver_data mt8195_vdosys1_driver_data = {
.ext_len = ARRAY_SIZE(mt8195_mtk_ddp_ext),
.mmsys_id = 1,
.mmsys_dev_num = 2,
+ .max_width = 8191,
+ .min_width = 2, /* 2-pixel align when ethdr is bypassed */
+ .min_height = 1,
};
static const struct of_device_id mtk_drm_of_ids[] = {
@@ -493,6 +502,15 @@ static int mtk_drm_kms_init(struct drm_device *drm)
for (j = 0; j < private->data->mmsys_dev_num; j++) {
priv_n = private->all_drm_private[j];
+ if (priv_n->data->max_width)
+ drm->mode_config.max_width = priv_n->data->max_width;
+
+ if (priv_n->data->min_width)
+ drm->mode_config.min_width = priv_n->data->min_width;
+
+ if (priv_n->data->min_height)
+ drm->mode_config.min_height = priv_n->data->min_height;
+
if (i == CRTC_MAIN && priv_n->data->main_len) {
ret = mtk_crtc_create(drm, priv_n->data->main_path,
priv_n->data->main_len, j,
@@ -520,6 +538,10 @@ static int mtk_drm_kms_init(struct drm_device *drm)
}
}
+ /* IGT will check if the cursor size is configured */
+ drm->mode_config.cursor_width = drm->mode_config.max_width;
+ drm->mode_config.cursor_height = drm->mode_config.max_height;
+
/* Use OVL device for all DMA memory allocations */
crtc = drm_crtc_from_index(drm, 0);
if (crtc)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index 78d698ede1bf8..6cfa790e8df5c 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -46,6 +46,10 @@ struct mtk_mmsys_driver_data {
bool shadow_register;
unsigned int mmsys_id;
unsigned int mmsys_dev_num;
+
+ int max_width;
+ int min_width;
+ int min_height;
};
struct mtk_drm_private {
--
2.18.0
Hi, Shawn:
On Thu, 2024-05-02 at 18:38 +0800, Shawn Sung wrote:
> From: Hsiao Chien Sung <[email protected]>
>
> Set DRM mode configs limitation according to the hardware
> capabilities
> and pass the IGT checks as below:
>
> - The test "graphics.IgtKms.kms_plane" requires a frame buffer with
> width of 4512 pixels (> 4096).
> - The test "graphics.IgtKms.kms_cursor_crc" checks if the cursor size
> is
> defined, and run the test with cursor size from 1x1 to 512x512.
>
> Please notice that the test conditions may change as IGT is updated.
Reviewed-by: CK Hu <[email protected]>
>
> Signed-off-by: Hsiao Chien Sung <[email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 22 ++++++++++++++++++++++
> drivers/gpu/drm/mediatek/mtk_drm_drv.h | 4 ++++
> 2 files changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 8e047043202b4..c9cad3a827376 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -294,6 +294,9 @@ static const struct mtk_mmsys_driver_data
> mt8188_vdosys0_driver_data = {
> .conn_routes = mt8188_mtk_ddp_main_routes,
> .num_conn_routes = ARRAY_SIZE(mt8188_mtk_ddp_main_routes),
> .mmsys_dev_num = 2,
> + .max_width = 8191,
> + .min_width = 1,
> + .min_height = 1,
> };
>
> static const struct mtk_mmsys_driver_data mt8192_mmsys_driver_data =
> {
> @@ -308,6 +311,9 @@ static const struct mtk_mmsys_driver_data
> mt8195_vdosys0_driver_data = {
> .main_path = mt8195_mtk_ddp_main,
> .main_len = ARRAY_SIZE(mt8195_mtk_ddp_main),
> .mmsys_dev_num = 2,
> + .max_width = 8191,
> + .min_width = 1,
> + .min_height = 1,
> };
>
> static const struct mtk_mmsys_driver_data mt8195_vdosys1_driver_data
> = {
> @@ -315,6 +321,9 @@ static const struct mtk_mmsys_driver_data
> mt8195_vdosys1_driver_data = {
> .ext_len = ARRAY_SIZE(mt8195_mtk_ddp_ext),
> .mmsys_id = 1,
> .mmsys_dev_num = 2,
> + .max_width = 8191,
> + .min_width = 2, /* 2-pixel align when ethdr is bypassed */
> + .min_height = 1,
> };
>
> static const struct of_device_id mtk_drm_of_ids[] = {
> @@ -493,6 +502,15 @@ static int mtk_drm_kms_init(struct drm_device
> *drm)
> for (j = 0; j < private->data->mmsys_dev_num; j++) {
> priv_n = private->all_drm_private[j];
>
> + if (priv_n->data->max_width)
> + drm->mode_config.max_width = priv_n-
> >data->max_width;
> +
> + if (priv_n->data->min_width)
> + drm->mode_config.min_width = priv_n-
> >data->min_width;
> +
> + if (priv_n->data->min_height)
> + drm->mode_config.min_height = priv_n-
> >data->min_height;
> +
> if (i == CRTC_MAIN && priv_n->data->main_len) {
> ret = mtk_crtc_create(drm, priv_n-
> >data->main_path,
> priv_n->data-
> >main_len, j,
> @@ -520,6 +538,10 @@ static int mtk_drm_kms_init(struct drm_device
> *drm)
> }
> }
>
> + /* IGT will check if the cursor size is configured */
> + drm->mode_config.cursor_width = drm->mode_config.max_width;
> + drm->mode_config.cursor_height = drm->mode_config.max_height;
> +
> /* Use OVL device for all DMA memory allocations */
> crtc = drm_crtc_from_index(drm, 0);
> if (crtc)
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> index 78d698ede1bf8..6cfa790e8df5c 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> @@ -46,6 +46,10 @@ struct mtk_mmsys_driver_data {
> bool shadow_register;
> unsigned int mmsys_id;
> unsigned int mmsys_dev_num;
> +
> + int max_width;
> + int min_width;
> + int min_height;
> };
>
> struct mtk_drm_private {
Hi Shawn,
On Thu, May 2, 2024 at 6:39 PM Shawn Sung <[email protected]> wrote:
>
> From: Hsiao Chien Sung <[email protected]>
>
> Set DRM mode configs limitation according to the hardware capabilities
> and pass the IGT checks as below:
>
> - The test "graphics.IgtKms.kms_plane" requires a frame buffer with
> width of 4512 pixels (> 4096).
> - The test "graphics.IgtKms.kms_cursor_crc" checks if the cursor size is
> defined, and run the test with cursor size from 1x1 to 512x512.
>
> Please notice that the test conditions may change as IGT is updated.
>
> Signed-off-by: Hsiao Chien Sung <[email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 22 ++++++++++++++++++++++
> drivers/gpu/drm/mediatek/mtk_drm_drv.h | 4 ++++
> 2 files changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 8e047043202b4..c9cad3a827376 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -294,6 +294,9 @@ static const struct mtk_mmsys_driver_data mt8188_vdosys0_driver_data = {
> .conn_routes = mt8188_mtk_ddp_main_routes,
> .num_conn_routes = ARRAY_SIZE(mt8188_mtk_ddp_main_routes),
> .mmsys_dev_num = 2,
> + .max_width = 8191,
> + .min_width = 1,
> + .min_height = 1,
> };
>
> static const struct mtk_mmsys_driver_data mt8192_mmsys_driver_data = {
> @@ -308,6 +311,9 @@ static const struct mtk_mmsys_driver_data mt8195_vdosys0_driver_data = {
> .main_path = mt8195_mtk_ddp_main,
> .main_len = ARRAY_SIZE(mt8195_mtk_ddp_main),
> .mmsys_dev_num = 2,
> + .max_width = 8191,
> + .min_width = 1,
> + .min_height = 1,
> };
>
> static const struct mtk_mmsys_driver_data mt8195_vdosys1_driver_data = {
> @@ -315,6 +321,9 @@ static const struct mtk_mmsys_driver_data mt8195_vdosys1_driver_data = {
> .ext_len = ARRAY_SIZE(mt8195_mtk_ddp_ext),
> .mmsys_id = 1,
> .mmsys_dev_num = 2,
> + .max_width = 8191,
> + .min_width = 2, /* 2-pixel align when ethdr is bypassed */
> + .min_height = 1,
> };
>
> static const struct of_device_id mtk_drm_of_ids[] = {
> @@ -493,6 +502,15 @@ static int mtk_drm_kms_init(struct drm_device *drm)
> for (j = 0; j < private->data->mmsys_dev_num; j++) {
> priv_n = private->all_drm_private[j];
>
> + if (priv_n->data->max_width)
> + drm->mode_config.max_width = priv_n->data->max_width;
> +
> + if (priv_n->data->min_width)
> + drm->mode_config.min_width = priv_n->data->min_width;
> +
> + if (priv_n->data->min_height)
> + drm->mode_config.min_height = priv_n->data->min_height;
> +
> if (i == CRTC_MAIN && priv_n->data->main_len) {
> ret = mtk_crtc_create(drm, priv_n->data->main_path,
> priv_n->data->main_len, j,
> @@ -520,6 +538,10 @@ static int mtk_drm_kms_init(struct drm_device *drm)
> }
> }
>
> + /* IGT will check if the cursor size is configured */
> + drm->mode_config.cursor_width = drm->mode_config.max_width;
> + drm->mode_config.cursor_height = drm->mode_config.max_height;
I think you shouldn't set the cursor size to the maximum plane size.
As you mentioned in the commit message, IGT tests the cursor sizes
from 1x1 to 512x512, so just setting them to 512 (with a macro) should
be enough.
Currently this tells the userspace "the supported cursor plane size is
up to 4096x8191" on MT8195 and MT8188.
That means the userspace may allocate a buffer with 4096*8191*4 ~=
128MB for the cursor plane in the worst case, and most of the buffer
will be wasted.
Note that the default value for DRM_CAP_CURSOR_WIDTH is 64.
In practice, when applying this on the MT8188 Chromebook, I see the
userspace allocates two 64MB buffers for the cursor plane.
But if I limit the cursor size to 512, the userspace only allocates
two 1MB buffers i.e. 126MB is optimized for the DMA buffer allocation.
Regards,
Fei
>
> +
> /* Use OVL device for all DMA memory allocations */
> crtc = drm_crtc_from_index(drm, 0);
> if (crtc)
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> index 78d698ede1bf8..6cfa790e8df5c 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> @@ -46,6 +46,10 @@ struct mtk_mmsys_driver_data {
> bool shadow_register;
> unsigned int mmsys_id;
> unsigned int mmsys_dev_num;
> +
> + int max_width;
> + int min_width;
> + int min_height;
> };
>
> struct mtk_drm_private {
> --
> 2.18.0
>
On Mon, May 27, 2024 at 12:34 PM Fei Shao <[email protected]> wrote:
>
> Hi Shawn,
>
> On Thu, May 2, 2024 at 6:39 PM Shawn Sung <[email protected]> wrote:
> >
> > From: Hsiao Chien Sung <[email protected]>
> >
> > Set DRM mode configs limitation according to the hardware capabilities
> > and pass the IGT checks as below:
> >
> > - The test "graphics.IgtKms.kms_plane" requires a frame buffer with
> > width of 4512 pixels (> 4096).
> > - The test "graphics.IgtKms.kms_cursor_crc" checks if the cursor size is
> > defined, and run the test with cursor size from 1x1 to 512x512.
> >
> > Please notice that the test conditions may change as IGT is updated.
> >
> > Signed-off-by: Hsiao Chien Sung <[email protected]>
> > ---
> > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 22 ++++++++++++++++++++++
> > drivers/gpu/drm/mediatek/mtk_drm_drv.h | 4 ++++
> > 2 files changed, 26 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > index 8e047043202b4..c9cad3a827376 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > @@ -294,6 +294,9 @@ static const struct mtk_mmsys_driver_data mt8188_vdosys0_driver_data = {
> > .conn_routes = mt8188_mtk_ddp_main_routes,
> > .num_conn_routes = ARRAY_SIZE(mt8188_mtk_ddp_main_routes),
> > .mmsys_dev_num = 2,
> > + .max_width = 8191,
> > + .min_width = 1,
> > + .min_height = 1,
> > };
> >
> > static const struct mtk_mmsys_driver_data mt8192_mmsys_driver_data = {
> > @@ -308,6 +311,9 @@ static const struct mtk_mmsys_driver_data mt8195_vdosys0_driver_data = {
> > .main_path = mt8195_mtk_ddp_main,
> > .main_len = ARRAY_SIZE(mt8195_mtk_ddp_main),
> > .mmsys_dev_num = 2,
> > + .max_width = 8191,
> > + .min_width = 1,
> > + .min_height = 1,
> > };
> >
> > static const struct mtk_mmsys_driver_data mt8195_vdosys1_driver_data = {
> > @@ -315,6 +321,9 @@ static const struct mtk_mmsys_driver_data mt8195_vdosys1_driver_data = {
> > .ext_len = ARRAY_SIZE(mt8195_mtk_ddp_ext),
> > .mmsys_id = 1,
> > .mmsys_dev_num = 2,
> > + .max_width = 8191,
> > + .min_width = 2, /* 2-pixel align when ethdr is bypassed */
> > + .min_height = 1,
> > };
> >
> > static const struct of_device_id mtk_drm_of_ids[] = {
> > @@ -493,6 +502,15 @@ static int mtk_drm_kms_init(struct drm_device *drm)
> > for (j = 0; j < private->data->mmsys_dev_num; j++) {
> > priv_n = private->all_drm_private[j];
> >
> > + if (priv_n->data->max_width)
> > + drm->mode_config.max_width = priv_n->data->max_width;
> > +
> > + if (priv_n->data->min_width)
> > + drm->mode_config.min_width = priv_n->data->min_width;
> > +
> > + if (priv_n->data->min_height)
> > + drm->mode_config.min_height = priv_n->data->min_height;
> > +
> > if (i == CRTC_MAIN && priv_n->data->main_len) {
> > ret = mtk_crtc_create(drm, priv_n->data->main_path,
> > priv_n->data->main_len, j,
> > @@ -520,6 +538,10 @@ static int mtk_drm_kms_init(struct drm_device *drm)
> > }
> > }
> >
> > + /* IGT will check if the cursor size is configured */
> > + drm->mode_config.cursor_width = drm->mode_config.max_width;
> > + drm->mode_config.cursor_height = drm->mode_config.max_height;
>
> I think you shouldn't set the cursor size to the maximum plane size.
> As you mentioned in the commit message, IGT tests the cursor sizes
> from 1x1 to 512x512, so just setting them to 512 (with a macro) should
> be enough.
>
> Currently this tells the userspace "the supported cursor plane size is
> up to 4096x8191" on MT8195 and MT8188.
> That means the userspace may allocate a buffer with 4096*8191*4 ~=
> 128MB for the cursor plane in the worst case, and most of the buffer
> will be wasted.
> Note that the default value for DRM_CAP_CURSOR_WIDTH is 64.
>
> In practice, when applying this on the MT8188 Chromebook, I see the
> userspace allocates two 64MB buffers for the cursor plane.
> But if I limit the cursor size to 512, the userspace only allocates
> two 1MB buffers i.e. 126MB is optimized for the DMA buffer allocation.
That seems more like an issue of ChromeOS not doing the most optimal
thing though?
> Regards,
> Fei
>
>
> >
> > +
> > /* Use OVL device for all DMA memory allocations */
> > crtc = drm_crtc_from_index(drm, 0);
> > if (crtc)
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > index 78d698ede1bf8..6cfa790e8df5c 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > @@ -46,6 +46,10 @@ struct mtk_mmsys_driver_data {
> > bool shadow_register;
> > unsigned int mmsys_id;
> > unsigned int mmsys_dev_num;
> > +
> > + int max_width;
> > + int min_width;
> > + int min_height;
> > };
> >
> > struct mtk_drm_private {
> > --
> > 2.18.0
> >
On Mon, May 27, 2024 at 12:38 PM Chen-Yu Tsai <[email protected]> wrote:
>
> On Mon, May 27, 2024 at 12:34 PM Fei Shao <[email protected]> wrote:
> >
> > Hi Shawn,
> >
> > On Thu, May 2, 2024 at 6:39 PM Shawn Sung <[email protected]> wrote:
> > >
> > > From: Hsiao Chien Sung <[email protected]>
> > >
> > > Set DRM mode configs limitation according to the hardware capabilities
> > > and pass the IGT checks as below:
> > >
> > > - The test "graphics.IgtKms.kms_plane" requires a frame buffer with
> > > width of 4512 pixels (> 4096).
> > > - The test "graphics.IgtKms.kms_cursor_crc" checks if the cursor size is
> > > defined, and run the test with cursor size from 1x1 to 512x512.
> > >
> > > Please notice that the test conditions may change as IGT is updated.
> > >
> > > Signed-off-by: Hsiao Chien Sung <[email protected]>
> > > ---
> > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 22 ++++++++++++++++++++++
> > > drivers/gpu/drm/mediatek/mtk_drm_drv.h | 4 ++++
> > > 2 files changed, 26 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > > index 8e047043202b4..c9cad3a827376 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > > @@ -294,6 +294,9 @@ static const struct mtk_mmsys_driver_data mt8188_vdosys0_driver_data = {
> > > .conn_routes = mt8188_mtk_ddp_main_routes,
> > > .num_conn_routes = ARRAY_SIZE(mt8188_mtk_ddp_main_routes),
> > > .mmsys_dev_num = 2,
> > > + .max_width = 8191,
> > > + .min_width = 1,
> > > + .min_height = 1,
> > > };
> > >
> > > static const struct mtk_mmsys_driver_data mt8192_mmsys_driver_data = {
> > > @@ -308,6 +311,9 @@ static const struct mtk_mmsys_driver_data mt8195_vdosys0_driver_data = {
> > > .main_path = mt8195_mtk_ddp_main,
> > > .main_len = ARRAY_SIZE(mt8195_mtk_ddp_main),
> > > .mmsys_dev_num = 2,
> > > + .max_width = 8191,
> > > + .min_width = 1,
> > > + .min_height = 1,
> > > };
> > >
> > > static const struct mtk_mmsys_driver_data mt8195_vdosys1_driver_data = {
> > > @@ -315,6 +321,9 @@ static const struct mtk_mmsys_driver_data mt8195_vdosys1_driver_data = {
> > > .ext_len = ARRAY_SIZE(mt8195_mtk_ddp_ext),
> > > .mmsys_id = 1,
> > > .mmsys_dev_num = 2,
> > > + .max_width = 8191,
> > > + .min_width = 2, /* 2-pixel align when ethdr is bypassed */
> > > + .min_height = 1,
> > > };
> > >
> > > static const struct of_device_id mtk_drm_of_ids[] = {
> > > @@ -493,6 +502,15 @@ static int mtk_drm_kms_init(struct drm_device *drm)
> > > for (j = 0; j < private->data->mmsys_dev_num; j++) {
> > > priv_n = private->all_drm_private[j];
> > >
> > > + if (priv_n->data->max_width)
> > > + drm->mode_config.max_width = priv_n->data->max_width;
> > > +
> > > + if (priv_n->data->min_width)
> > > + drm->mode_config.min_width = priv_n->data->min_width;
> > > +
> > > + if (priv_n->data->min_height)
> > > + drm->mode_config.min_height = priv_n->data->min_height;
> > > +
> > > if (i == CRTC_MAIN && priv_n->data->main_len) {
> > > ret = mtk_crtc_create(drm, priv_n->data->main_path,
> > > priv_n->data->main_len, j,
> > > @@ -520,6 +538,10 @@ static int mtk_drm_kms_init(struct drm_device *drm)
> > > }
> > > }
> > >
> > > + /* IGT will check if the cursor size is configured */
> > > + drm->mode_config.cursor_width = drm->mode_config.max_width;
> > > + drm->mode_config.cursor_height = drm->mode_config.max_height;
> >
> > I think you shouldn't set the cursor size to the maximum plane size.
> > As you mentioned in the commit message, IGT tests the cursor sizes
> > from 1x1 to 512x512, so just setting them to 512 (with a macro) should
> > be enough.
> >
> > Currently this tells the userspace "the supported cursor plane size is
> > up to 4096x8191" on MT8195 and MT8188.
> > That means the userspace may allocate a buffer with 4096*8191*4 ~=
> > 128MB for the cursor plane in the worst case, and most of the buffer
> > will be wasted.
> > Note that the default value for DRM_CAP_CURSOR_WIDTH is 64.
> >
> > In practice, when applying this on the MT8188 Chromebook, I see the
> > userspace allocates two 64MB buffers for the cursor plane.
> > But if I limit the cursor size to 512, the userspace only allocates
> > two 1MB buffers i.e. 126MB is optimized for the DMA buffer allocation.
>
> That seems more like an issue of ChromeOS not doing the most optimal
> thing though?
I agree that we need to fix this on the ChromeOS side too, but I still
think the driver doesn't need to set the capabilities like this
because a full-screen size cursor is not practical anyway.
The default cursor plane size is just 64x64.
Regard,
Fei
>
> > Regards,
> > Fei
> >
> >
> > >
> > > +
> > > /* Use OVL device for all DMA memory allocations */
> > > crtc = drm_crtc_from_index(drm, 0);
> > > if (crtc)
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > > index 78d698ede1bf8..6cfa790e8df5c 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > > @@ -46,6 +46,10 @@ struct mtk_mmsys_driver_data {
> > > bool shadow_register;
> > > unsigned int mmsys_id;
> > > unsigned int mmsys_dev_num;
> > > +
> > > + int max_width;
> > > + int min_width;
> > > + int min_height;
> > > };
> > >
> > > struct mtk_drm_private {
> > > --
> > > 2.18.0
> > >
On Mon, May 27, 2024 at 12:54 PM Fei Shao <[email protected]> wrote:
>
> On Mon, May 27, 2024 at 12:38 PM Chen-Yu Tsai <[email protected]> wrote:
> >
> > On Mon, May 27, 2024 at 12:34 PM Fei Shao <[email protected]> wrote:
> > >
> > > Hi Shawn,
> > >
> > > On Thu, May 2, 2024 at 6:39 PM Shawn Sung <[email protected]> wrote:
> > > >
> > > > From: Hsiao Chien Sung <[email protected]>
> > > >
> > > > Set DRM mode configs limitation according to the hardware capabilities
> > > > and pass the IGT checks as below:
> > > >
> > > > - The test "graphics.IgtKms.kms_plane" requires a frame buffer with
> > > > width of 4512 pixels (> 4096).
> > > > - The test "graphics.IgtKms.kms_cursor_crc" checks if the cursor size is
> > > > defined, and run the test with cursor size from 1x1 to 512x512.
> > > >
> > > > Please notice that the test conditions may change as IGT is updated.
> > > >
> > > > Signed-off-by: Hsiao Chien Sung <[email protected]>
> > > > ---
> > > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 22 ++++++++++++++++++++++
> > > > drivers/gpu/drm/mediatek/mtk_drm_drv.h | 4 ++++
> > > > 2 files changed, 26 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > > > index 8e047043202b4..c9cad3a827376 100644
> > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > > > @@ -294,6 +294,9 @@ static const struct mtk_mmsys_driver_data mt8188_vdosys0_driver_data = {
> > > > .conn_routes = mt8188_mtk_ddp_main_routes,
> > > > .num_conn_routes = ARRAY_SIZE(mt8188_mtk_ddp_main_routes),
> > > > .mmsys_dev_num = 2,
> > > > + .max_width = 8191,
> > > > + .min_width = 1,
> > > > + .min_height = 1,
> > > > };
> > > >
> > > > static const struct mtk_mmsys_driver_data mt8192_mmsys_driver_data = {
> > > > @@ -308,6 +311,9 @@ static const struct mtk_mmsys_driver_data mt8195_vdosys0_driver_data = {
> > > > .main_path = mt8195_mtk_ddp_main,
> > > > .main_len = ARRAY_SIZE(mt8195_mtk_ddp_main),
> > > > .mmsys_dev_num = 2,
> > > > + .max_width = 8191,
> > > > + .min_width = 1,
> > > > + .min_height = 1,
> > > > };
> > > >
> > > > static const struct mtk_mmsys_driver_data mt8195_vdosys1_driver_data = {
> > > > @@ -315,6 +321,9 @@ static const struct mtk_mmsys_driver_data mt8195_vdosys1_driver_data = {
> > > > .ext_len = ARRAY_SIZE(mt8195_mtk_ddp_ext),
> > > > .mmsys_id = 1,
> > > > .mmsys_dev_num = 2,
> > > > + .max_width = 8191,
> > > > + .min_width = 2, /* 2-pixel align when ethdr is bypassed */
> > > > + .min_height = 1,
> > > > };
> > > >
> > > > static const struct of_device_id mtk_drm_of_ids[] = {
> > > > @@ -493,6 +502,15 @@ static int mtk_drm_kms_init(struct drm_device *drm)
> > > > for (j = 0; j < private->data->mmsys_dev_num; j++) {
> > > > priv_n = private->all_drm_private[j];
> > > >
> > > > + if (priv_n->data->max_width)
> > > > + drm->mode_config.max_width = priv_n->data->max_width;
> > > > +
> > > > + if (priv_n->data->min_width)
> > > > + drm->mode_config.min_width = priv_n->data->min_width;
> > > > +
> > > > + if (priv_n->data->min_height)
> > > > + drm->mode_config.min_height = priv_n->data->min_height;
> > > > +
> > > > if (i == CRTC_MAIN && priv_n->data->main_len) {
> > > > ret = mtk_crtc_create(drm, priv_n->data->main_path,
> > > > priv_n->data->main_len, j,
> > > > @@ -520,6 +538,10 @@ static int mtk_drm_kms_init(struct drm_device *drm)
> > > > }
> > > > }
> > > >
> > > > + /* IGT will check if the cursor size is configured */
> > > > + drm->mode_config.cursor_width = drm->mode_config.max_width;
> > > > + drm->mode_config.cursor_height = drm->mode_config.max_height;
> > >
> > > I think you shouldn't set the cursor size to the maximum plane size.
> > > As you mentioned in the commit message, IGT tests the cursor sizes
> > > from 1x1 to 512x512, so just setting them to 512 (with a macro) should
> > > be enough.
> > >
> > > Currently this tells the userspace "the supported cursor plane size is
> > > up to 4096x8191" on MT8195 and MT8188.
> > > That means the userspace may allocate a buffer with 4096*8191*4 ~=
> > > 128MB for the cursor plane in the worst case, and most of the buffer
> > > will be wasted.
> > > Note that the default value for DRM_CAP_CURSOR_WIDTH is 64.
> > >
> > > In practice, when applying this on the MT8188 Chromebook, I see the
> > > userspace allocates two 64MB buffers for the cursor plane.
> > > But if I limit the cursor size to 512, the userspace only allocates
> > > two 1MB buffers i.e. 126MB is optimized for the DMA buffer allocation.
> >
> > That seems more like an issue of ChromeOS not doing the most optimal
> > thing though?
>
> I agree that we need to fix this on the ChromeOS side too, but I still
> think the driver doesn't need to set the capabilities like this
> because a full-screen size cursor is not practical anyway.
> The default cursor plane size is just 64x64.
Seems like a better solution was merged:
https://patchwork.freedesktop.org/patch/583298/
ChenYu
> Regard,
> Fei
>
>
>
> >
> > > Regards,
> > > Fei
> > >
> > >
> > > >
> > > > +
> > > > /* Use OVL device for all DMA memory allocations */
> > > > crtc = drm_crtc_from_index(drm, 0);
> > > > if (crtc)
> > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > > > index 78d698ede1bf8..6cfa790e8df5c 100644
> > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > > > @@ -46,6 +46,10 @@ struct mtk_mmsys_driver_data {
> > > > bool shadow_register;
> > > > unsigned int mmsys_id;
> > > > unsigned int mmsys_dev_num;
> > > > +
> > > > + int max_width;
> > > > + int min_width;
> > > > + int min_height;
> > > > };
> > > >
> > > > struct mtk_drm_private {
> > > > --
> > > > 2.18.0
> > > >