* [PATCH] drm/i915: Check fb stride against plane max stride
@ 2018-09-18 14:02 Ville Syrjala
2018-09-18 15:12 ` ✓ Fi.CI.BAT: success for " Patchwork
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Ville Syrjala @ 2018-09-18 14:02 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
commit 4e0b83a567e2 ("drm/i915: Extract per-platform plane->check()
functions") removed the plane max stride check for sprite planes.
I was going to add it back when introducing GTT remapping for the
display, but after further thought it seems better to re-introduce
it separately.
So let's add the max stride check back. And let's do it in a nicer
form than what we had before and do it for all plane types (easy
now that we have the ->max_stride() plane vfunc).
Only sprite planes really need this for now since primary planes
are capable of scanning out the current max fb size we allow, and
cursors have more stringent stride checks elsewhere.
Cc: José Roberto de Souza <jose.souza@intel.com>
Fixes: 4e0b83a567e2 ("drm/i915: Extract per-platform plane->check() functions")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++++++
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_sprite.c | 22 ++++++++++++++++++++++
3 files changed, 37 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index eb25037d7b38..1eb99d5ec221 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3151,6 +3151,10 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state)
plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0, rotation);
plane_state->color_plane[1].stride = intel_fb_pitch(fb, 1, rotation);
+ ret = intel_plane_check_stride(plane_state);
+ if (ret)
+ return ret;
+
if (!plane_state->base.visible)
return 0;
@@ -3286,10 +3290,15 @@ int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
int src_x = plane_state->base.src.x1 >> 16;
int src_y = plane_state->base.src.y1 >> 16;
u32 offset;
+ int ret;
intel_fill_fb_ggtt_view(&plane_state->view, fb, rotation);
plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0, rotation);
+ ret = intel_plane_check_stride(plane_state);
+ if (ret)
+ return ret;
+
intel_add_fb_offsets(&src_x, &src_y, plane_state, 0);
if (INTEL_GEN(dev_priv) >= 4)
@@ -9685,10 +9694,15 @@ static int intel_cursor_check_surface(struct intel_plane_state *plane_state)
unsigned int rotation = plane_state->base.rotation;
int src_x, src_y;
u32 offset;
+ int ret;
intel_fill_fb_ggtt_view(&plane_state->view, fb, rotation);
plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0, rotation);
+ ret = intel_plane_check_stride(plane_state);
+ if (ret)
+ return ret;
+
src_x = plane_state->base.src_x >> 16;
src_y = plane_state->base.src_y >> 16;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bf1c38728a59..a34c2f1f9159 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2140,6 +2140,7 @@ unsigned int skl_plane_max_stride(struct intel_plane *plane,
unsigned int rotation);
int skl_plane_check(struct intel_crtc_state *crtc_state,
struct intel_plane_state *plane_state);
+int intel_plane_check_stride(const struct intel_plane_state *plane_state);
int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state);
int chv_plane_check_rotation(const struct intel_plane_state *plane_state);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index d4c8e10fc90b..5fd2f7bf3927 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -230,6 +230,28 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
#endif
}
+int intel_plane_check_stride(const struct intel_plane_state *plane_state)
+{
+ struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
+ const struct drm_framebuffer *fb = plane_state->base.fb;
+ unsigned int rotation = plane_state->base.rotation;
+ u32 stride, max_stride;
+
+ /* FIXME other color planes? */
+ stride = plane_state->color_plane[0].stride;
+ max_stride = plane->max_stride(plane, fb->format->format,
+ fb->modifier, rotation);
+
+ if (stride > max_stride) {
+ DRM_DEBUG_KMS("[FB:%d] stride (%d) exceeds [PLANE:%d:%s] max stride (%d)\n",
+ fb->base.id, stride,
+ plane->base.base.id, plane->base.name, max_stride);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state)
{
const struct drm_framebuffer *fb = plane_state->base.fb;
--
2.16.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* ✓ Fi.CI.BAT: success for drm/i915: Check fb stride against plane max stride
2018-09-18 14:02 [PATCH] drm/i915: Check fb stride against plane max stride Ville Syrjala
@ 2018-09-18 15:12 ` Patchwork
2018-09-18 17:32 ` ✓ Fi.CI.IGT: " Patchwork
2018-09-21 22:17 ` [PATCH] " Dhinakaran Pandiyan
2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-09-18 15:12 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Check fb stride against plane max stride
URL : https://patchwork.freedesktop.org/series/49844/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4837 -> Patchwork_10213 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/49844/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_10213 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@drv_selftest@mock_hugepages:
fi-bwr-2160: PASS -> DMESG-FAIL (fdo#107930)
igt@kms_frontbuffer_tracking@basic:
fi-byt-clapper: PASS -> FAIL (fdo#103167)
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
fi-byt-clapper: PASS -> FAIL (fdo#107362, fdo#103191)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
fi-bdw-samus: NOTRUN -> INCOMPLETE (fdo#107773)
fi-icl-u: PASS -> INCOMPLETE (fdo#107713)
==== Possible fixes ====
igt@gem_exec_suspend@basic-s3:
fi-bdw-samus: INCOMPLETE (fdo#107773) -> PASS
fi-blb-e6850: INCOMPLETE (fdo#107718) -> PASS
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107713 https://bugs.freedesktop.org/show_bug.cgi?id=107713
fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
fdo#107930 https://bugs.freedesktop.org/show_bug.cgi?id=107930
== Participating hosts (51 -> 45) ==
Missing (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-glk-j4005
== Build changes ==
* Linux: CI_DRM_4837 -> Patchwork_10213
CI_DRM_4837: 09b295662edd3982c39d4ec4eba87483cbc1d9dc @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4646: d409cc6f234fbc0122c64be27ba85b5603658de5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10213: 04df6609a035b1a97fa7118c80999285249f05af @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
04df6609a035 drm/i915: Check fb stride against plane max stride
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10213/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* ✓ Fi.CI.IGT: success for drm/i915: Check fb stride against plane max stride
2018-09-18 14:02 [PATCH] drm/i915: Check fb stride against plane max stride Ville Syrjala
2018-09-18 15:12 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-09-18 17:32 ` Patchwork
2018-09-21 22:17 ` [PATCH] " Dhinakaran Pandiyan
2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-09-18 17:32 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Check fb stride against plane max stride
URL : https://patchwork.freedesktop.org/series/49844/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4837_full -> Patchwork_10213_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_10213_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_10213_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_10213_full:
=== IGT changes ===
==== Warnings ====
igt@kms_frontbuffer_tracking@fbc-1p-indfb-fliptrack:
shard-snb: INCOMPLETE (fdo#105411) -> DMESG-FAIL
== Known issues ==
Here are the changes found in Patchwork_10213_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_exec_schedule@pi-ringfull-bsd2:
shard-kbl: NOTRUN -> FAIL (fdo#103158)
igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c:
shard-kbl: NOTRUN -> DMESG-WARN (fdo#107956)
igt@kms_busy@extended-pageflip-hang-newfb-render-b:
shard-apl: NOTRUN -> DMESG-WARN (fdo#107956)
igt@kms_chv_cursor_fail@pipe-a-64x64-bottom-edge:
shard-glk: PASS -> FAIL (fdo#104671)
igt@kms_cursor_legacy@cursora-vs-flipa-toggle:
shard-glk: PASS -> DMESG-WARN (fdo#105763, fdo#106538)
igt@kms_draw_crc@draw-method-xrgb2101010-mmap-gtt-xtiled:
shard-glk: PASS -> FAIL (fdo#103184)
igt@kms_flip@2x-plain-flip-fb-recreate-interruptible:
shard-hsw: PASS -> DMESG-WARN (fdo#102614)
igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-render:
shard-glk: PASS -> FAIL (fdo#103167)
igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
shard-snb: NOTRUN -> INCOMPLETE (fdo#105411)
igt@syncobj_wait@multi-wait-all-for-submit-submitted:
shard-snb: PASS -> DMESG-WARN (fdo#107469)
==== Possible fixes ====
igt@gem_render_linear_blits@basic:
shard-kbl: INCOMPLETE (fdo#103665) -> PASS
igt@kms_flip@flip-vs-expired-vblank-interruptible:
shard-glk: FAIL (fdo#105363) -> PASS
igt@kms_setmode@basic:
shard-apl: FAIL (fdo#99912) -> PASS
shard-kbl: FAIL (fdo#99912) -> PASS
igt@pm_rpm@system-suspend-execbuf:
shard-apl: INCOMPLETE (fdo#103927) -> PASS
fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
fdo#107469 https://bugs.freedesktop.org/show_bug.cgi?id=107469
fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
== Participating hosts (5 -> 5) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_4837 -> Patchwork_10213
CI_DRM_4837: 09b295662edd3982c39d4ec4eba87483cbc1d9dc @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4646: d409cc6f234fbc0122c64be27ba85b5603658de5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10213: 04df6609a035b1a97fa7118c80999285249f05af @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10213/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/i915: Check fb stride against plane max stride
2018-09-18 14:02 [PATCH] drm/i915: Check fb stride against plane max stride Ville Syrjala
2018-09-18 15:12 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-09-18 17:32 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-09-21 22:17 ` Dhinakaran Pandiyan
2018-09-24 13:31 ` Ville Syrjälä
2 siblings, 1 reply; 6+ messages in thread
From: Dhinakaran Pandiyan @ 2018-09-21 22:17 UTC (permalink / raw)
To: intel-gfx; +Cc: dhinakaran.pandiyan
On Tuesday, September 18, 2018 7:02:43 AM PDT Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> commit 4e0b83a567e2 ("drm/i915: Extract per-platform plane->check()
> functions") removed the plane max stride check for sprite planes.
> I was going to add it back when introducing GTT remapping for the
> display, but after further thought it seems better to re-introduce
> it separately.
>
> So let's add the max stride check back. And let's do it in a nicer
> form than what we had before and do it for all plane types (easy
> now that we have the ->max_stride() plane vfunc).
>
> Only sprite planes really need this for now since primary planes
> are capable of scanning out the current max fb size we allow, and
> cursors have more stringent stride checks elsewhere.
>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Fixes: 4e0b83a567e2 ("drm/i915: Extract per-platform plane->check()
> functions") Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++++++
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_sprite.c | 22 ++++++++++++++++++++++
> 3 files changed, 37 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c index eb25037d7b38..1eb99d5ec221
> 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3151,6 +3151,10 @@ int skl_check_plane_surface(struct intel_plane_state
> *plane_state) plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0,
> rotation); plane_state->color_plane[1].stride = intel_fb_pitch(fb, 1,
> rotation);
>
> + ret = intel_plane_check_stride(plane_state);
> + if (ret)
> + return ret;
> +
> if (!plane_state->base.visible)
> return 0;
>
> @@ -3286,10 +3290,15 @@ int i9xx_check_plane_surface(struct
> intel_plane_state *plane_state) int src_x = plane_state->base.src.x1 >> 16;
> int src_y = plane_state->base.src.y1 >> 16;
> u32 offset;
> + int ret;
>
> intel_fill_fb_ggtt_view(&plane_state->view, fb, rotation);
> plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0, rotation);
>
Is there a good reason to not inline the code ?
if (color_plane[0].stride > plane->max_stride())
return -EINVAL;
> + ret = intel_plane_check_stride(plane_state);
> + if (ret)
> + return ret;
> +
> intel_add_fb_offsets(&src_x, &src_y, plane_state, 0);
>
> if (INTEL_GEN(dev_priv) >= 4)
> @@ -9685,10 +9694,15 @@ static int intel_cursor_check_surface(struct
> intel_plane_state *plane_state) unsigned int rotation =
> plane_state->base.rotation;
> int src_x, src_y;
> u32 offset;
> + int ret;
>
> intel_fill_fb_ggtt_view(&plane_state->view, fb, rotation);
> plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0, rotation);
>
> + ret = intel_plane_check_stride(plane_state);
> + if (ret)
> + return ret;
> +
> src_x = plane_state->base.src_x >> 16;
> src_y = plane_state->base.src_y >> 16;
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h index bf1c38728a59..a34c2f1f9159 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2140,6 +2140,7 @@ unsigned int skl_plane_max_stride(struct intel_plane
> *plane, unsigned int rotation);
> int skl_plane_check(struct intel_crtc_state *crtc_state,
> struct intel_plane_state *plane_state);
> +int intel_plane_check_stride(const struct intel_plane_state *plane_state);
> int intel_plane_check_src_coordinates(struct intel_plane_state
> *plane_state); int chv_plane_check_rotation(const struct intel_plane_state
> *plane_state);
>
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c index d4c8e10fc90b..5fd2f7bf3927
> 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -230,6 +230,28 @@ void intel_pipe_update_end(struct intel_crtc_state
> *new_crtc_state) #endif
> }
>
> +int intel_plane_check_stride(const struct intel_plane_state *plane_state)
> +{
> + struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
> + const struct drm_framebuffer *fb = plane_state->base.fb;
> + unsigned int rotation = plane_state->base.rotation;
> + u32 stride, max_stride;
> +
> + /* FIXME other color planes? */
Doesn't the color plane 0 have the max stride always?
> + stride = plane_state->color_plane[0].stride;
> + max_stride = plane->max_stride(plane, fb->format->format,
> + fb->modifier, rotation);
> +
> + if (stride > max_stride) {
> + DRM_DEBUG_KMS("[FB:%d] stride (%d) exceeds [PLANE:%d:%s] max stride
> (%d)\n", + fb->base.id, stride,
> + plane->base.base.id, plane->base.name, max_stride);
Include the color plane number in the debug print?
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> int intel_plane_check_src_coordinates(struct intel_plane_state
> *plane_state) {
> const struct drm_framebuffer *fb = plane_state->base.fb;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/i915: Check fb stride against plane max stride
2018-09-21 22:17 ` [PATCH] " Dhinakaran Pandiyan
@ 2018-09-24 13:31 ` Ville Syrjälä
2018-09-24 18:56 ` Dhinakaran Pandiyan
0 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2018-09-24 13:31 UTC (permalink / raw)
To: Dhinakaran Pandiyan; +Cc: intel-gfx, dhinakaran.pandiyan
On Fri, Sep 21, 2018 at 03:17:45PM -0700, Dhinakaran Pandiyan wrote:
> On Tuesday, September 18, 2018 7:02:43 AM PDT Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > commit 4e0b83a567e2 ("drm/i915: Extract per-platform plane->check()
> > functions") removed the plane max stride check for sprite planes.
> > I was going to add it back when introducing GTT remapping for the
> > display, but after further thought it seems better to re-introduce
> > it separately.
> >
> > So let's add the max stride check back. And let's do it in a nicer
> > form than what we had before and do it for all plane types (easy
> > now that we have the ->max_stride() plane vfunc).
> >
> > Only sprite planes really need this for now since primary planes
> > are capable of scanning out the current max fb size we allow, and
> > cursors have more stringent stride checks elsewhere.
> >
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Fixes: 4e0b83a567e2 ("drm/i915: Extract per-platform plane->check()
> > functions") Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++++++
> > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > drivers/gpu/drm/i915/intel_sprite.c | 22 ++++++++++++++++++++++
> > 3 files changed, 37 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c index eb25037d7b38..1eb99d5ec221
> > 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3151,6 +3151,10 @@ int skl_check_plane_surface(struct intel_plane_state
> > *plane_state) plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0,
> > rotation); plane_state->color_plane[1].stride = intel_fb_pitch(fb, 1,
> > rotation);
> >
> > + ret = intel_plane_check_stride(plane_state);
> > + if (ret)
> > + return ret;
> > +
> > if (!plane_state->base.visible)
> > return 0;
> >
> > @@ -3286,10 +3290,15 @@ int i9xx_check_plane_surface(struct
> > intel_plane_state *plane_state) int src_x = plane_state->base.src.x1 >> 16;
> > int src_y = plane_state->base.src.y1 >> 16;
> > u32 offset;
> > + int ret;
> >
> > intel_fill_fb_ggtt_view(&plane_state->view, fb, rotation);
> > plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0, rotation);
> >
> Is there a good reason to not inline the code ?
> if (color_plane[0].stride > plane->max_stride())
> return -EINVAL;
Consistency. Easier to just call a single function that does things in a
consistent fashion rather than duplicating the same check everywhere.
Also keeps the debug print consistent.
>
>
> > + ret = intel_plane_check_stride(plane_state);
> > + if (ret)
> > + return ret;
> > +
> > intel_add_fb_offsets(&src_x, &src_y, plane_state, 0);
> >
> > if (INTEL_GEN(dev_priv) >= 4)
> > @@ -9685,10 +9694,15 @@ static int intel_cursor_check_surface(struct
> > intel_plane_state *plane_state) unsigned int rotation =
> > plane_state->base.rotation;
> > int src_x, src_y;
> > u32 offset;
> > + int ret;
> >
> > intel_fill_fb_ggtt_view(&plane_state->view, fb, rotation);
> > plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0, rotation);
> >
> > + ret = intel_plane_check_stride(plane_state);
> > + if (ret)
> > + return ret;
> > +
> > src_x = plane_state->base.src_x >> 16;
> > src_y = plane_state->base.src_y >> 16;
> >
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h index bf1c38728a59..a34c2f1f9159 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -2140,6 +2140,7 @@ unsigned int skl_plane_max_stride(struct intel_plane
> > *plane, unsigned int rotation);
> > int skl_plane_check(struct intel_crtc_state *crtc_state,
> > struct intel_plane_state *plane_state);
> > +int intel_plane_check_stride(const struct intel_plane_state *plane_state);
> > int intel_plane_check_src_coordinates(struct intel_plane_state
> > *plane_state); int chv_plane_check_rotation(const struct intel_plane_state
> > *plane_state);
> >
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> > b/drivers/gpu/drm/i915/intel_sprite.c index d4c8e10fc90b..5fd2f7bf3927
> > 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -230,6 +230,28 @@ void intel_pipe_update_end(struct intel_crtc_state
> > *new_crtc_state) #endif
> > }
> >
> > +int intel_plane_check_stride(const struct intel_plane_state *plane_state)
> > +{
> > + struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
> > + const struct drm_framebuffer *fb = plane_state->base.fb;
> > + unsigned int rotation = plane_state->base.rotation;
> > + u32 stride, max_stride;
> > +
> > + /* FIXME other color planes? */
> Doesn't the color plane 0 have the max stride always?
Each color plane could have a different maximum. Depends on the hw.
Not that it's actually documented for the skl+ aux surface. Would
probably need to test what is the max, or just pick a sensible
limit that still seems to work.
>
>
> > + stride = plane_state->color_plane[0].stride;
> > + max_stride = plane->max_stride(plane, fb->format->format,
> > + fb->modifier, rotation);
> > +
> > + if (stride > max_stride) {
> > + DRM_DEBUG_KMS("[FB:%d] stride (%d) exceeds [PLANE:%d:%s] max stride
> > (%d)\n", + fb->base.id, stride,
> > + plane->base.base.id, plane->base.name, max_stride);
> Include the color plane number in the debug print?
I suppose.
>
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > int intel_plane_check_src_coordinates(struct intel_plane_state
> > *plane_state) {
> > const struct drm_framebuffer *fb = plane_state->base.fb;
>
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/i915: Check fb stride against plane max stride
2018-09-24 13:31 ` Ville Syrjälä
@ 2018-09-24 18:56 ` Dhinakaran Pandiyan
0 siblings, 0 replies; 6+ messages in thread
From: Dhinakaran Pandiyan @ 2018-09-24 18:56 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On Monday, September 24, 2018 6:31:56 AM PDT Ville Syrjälä wrote:
> On Fri, Sep 21, 2018 at 03:17:45PM -0700, Dhinakaran Pandiyan wrote:
> > On Tuesday, September 18, 2018 7:02:43 AM PDT Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > commit 4e0b83a567e2 ("drm/i915: Extract per-platform plane->check()
> > > functions") removed the plane max stride check for sprite planes.
> > > I was going to add it back when introducing GTT remapping for the
> > > display, but after further thought it seems better to re-introduce
> > > it separately.
> > >
> > > So let's add the max stride check back. And let's do it in a nicer
> > > form than what we had before and do it for all plane types (easy
> > > now that we have the ->max_stride() plane vfunc).
> > >
> > > Only sprite planes really need this for now since primary planes
> > > are capable of scanning out the current max fb size we allow, and
> > > cursors have more stringent stride checks elsewhere.
> > >
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: José Roberto de Souza <jose.souza@intel.com>
> > > Fixes: 4e0b83a567e2 ("drm/i915: Extract per-platform plane->check()
> > > functions") Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >
> > > drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++++++
> > > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > > drivers/gpu/drm/i915/intel_sprite.c | 22 ++++++++++++++++++++++
> > > 3 files changed, 37 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c index eb25037d7b38..1eb99d5ec221
> > > 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -3151,6 +3151,10 @@ int skl_check_plane_surface(struct
> > > intel_plane_state
> > > *plane_state) plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0,
> > > rotation); plane_state->color_plane[1].stride = intel_fb_pitch(fb, 1,
> > > rotation);
> > >
> > > + ret = intel_plane_check_stride(plane_state);
> > > + if (ret)
> > > + return ret;
> > > +
> > >
> > > if (!plane_state->base.visible)
> > >
> > > return 0;
> > >
> > > @@ -3286,10 +3290,15 @@ int i9xx_check_plane_surface(struct
> > > intel_plane_state *plane_state) int src_x = plane_state->base.src.x1 >>
> > > 16;
> > >
> > > int src_y = plane_state->base.src.y1 >> 16;
> > > u32 offset;
> > >
> > > + int ret;
> > >
> > > intel_fill_fb_ggtt_view(&plane_state->view, fb, rotation);
> > > plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0, rotation);
> >
> > Is there a good reason to not inline the code ?
> >
> > if (color_plane[0].stride > plane->max_stride())
> >
> > return -EINVAL;
>
> Consistency. Easier to just call a single function that does things in a
> consistent fashion rather than duplicating the same check everywhere.
> Also keeps the debug print consistent.
>
> > > + ret = intel_plane_check_stride(plane_state);
> > > + if (ret)
> > > + return ret;
> > > +
> > >
> > > intel_add_fb_offsets(&src_x, &src_y, plane_state, 0);
> > >
> > > if (INTEL_GEN(dev_priv) >= 4)
> > >
> > > @@ -9685,10 +9694,15 @@ static int intel_cursor_check_surface(struct
> > > intel_plane_state *plane_state) unsigned int rotation =
> > > plane_state->base.rotation;
> > >
> > > int src_x, src_y;
> > > u32 offset;
> > >
> > > + int ret;
> > >
> > > intel_fill_fb_ggtt_view(&plane_state->view, fb, rotation);
> > > plane_state->color_plane[0].stride = intel_fb_pitch(fb, 0, rotation);
> > >
> > > + ret = intel_plane_check_stride(plane_state);
> > > + if (ret)
> > > + return ret;
> > > +
> > >
> > > src_x = plane_state->base.src_x >> 16;
> > > src_y = plane_state->base.src_y >> 16;
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > b/drivers/gpu/drm/i915/intel_drv.h index bf1c38728a59..a34c2f1f9159
> > > 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -2140,6 +2140,7 @@ unsigned int skl_plane_max_stride(struct
> > > intel_plane
> > > *plane, unsigned int rotation);
> > >
> > > int skl_plane_check(struct intel_crtc_state *crtc_state,
> > >
> > > struct intel_plane_state *plane_state);
> > >
> > > +int intel_plane_check_stride(const struct intel_plane_state
> > > *plane_state);
> > >
> > > int intel_plane_check_src_coordinates(struct intel_plane_state
> > >
> > > *plane_state); int chv_plane_check_rotation(const struct
> > > intel_plane_state
> > > *plane_state);
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> > > b/drivers/gpu/drm/i915/intel_sprite.c index d4c8e10fc90b..5fd2f7bf3927
> > > 100644
> > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > @@ -230,6 +230,28 @@ void intel_pipe_update_end(struct intel_crtc_state
> > > *new_crtc_state) #endif
> > >
> > > }
> > >
> > > +int intel_plane_check_stride(const struct intel_plane_state
> > > *plane_state)
> > > +{
> > > + struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
> > > + const struct drm_framebuffer *fb = plane_state->base.fb;
> > > + unsigned int rotation = plane_state->base.rotation;
> > > + u32 stride, max_stride;
> > > +
> > > + /* FIXME other color planes? */
> >
> > Doesn't the color plane 0 have the max stride always?
>
> Each color plane could have a different maximum. Depends on the hw.
> Not that it's actually documented for the skl+ aux surface. Would
> probably need to test what is the max, or just pick a sensible
> limit that still seems to work.
>
> > > + stride = plane_state->color_plane[0].stride;
> > > + max_stride = plane->max_stride(plane, fb->format->format,
> > > + fb->modifier, rotation);
> > > +
> > > + if (stride > max_stride) {
> > > + DRM_DEBUG_KMS("[FB:%d] stride (%d) exceeds [PLANE:%d:%s] max stride
> > > (%d)\n", + fb->base.id, stride,
> > > + plane->base.base.id, plane->base.name, max_stride);
> >
> > Include the color plane number in the debug print?
>
> I suppose.
>
> > > + return -EINVAL;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > >
> > > int intel_plane_check_src_coordinates(struct intel_plane_state
> > >
> > > *plane_state) {
> > >
> > > const struct drm_framebuffer *fb = plane_state->base.fb;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-09-24 18:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-18 14:02 [PATCH] drm/i915: Check fb stride against plane max stride Ville Syrjala
2018-09-18 15:12 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-09-18 17:32 ` ✓ Fi.CI.IGT: " Patchwork
2018-09-21 22:17 ` [PATCH] " Dhinakaran Pandiyan
2018-09-24 13:31 ` Ville Syrjälä
2018-09-24 18:56 ` Dhinakaran Pandiyan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.