* [PATCH 2/3] drm/i915: make the primary plane func structs const
2018-09-25 0:19 [PATCH 1/3] drm/i915: DRM_FORMAT_C8 is not possible with Yf tiling Paulo Zanoni
@ 2018-09-25 0:19 ` Paulo Zanoni
2018-09-25 12:05 ` Ville Syrjälä
2018-09-25 0:19 ` [PATCH 3/3] drm/i915: remove a copy of skl_plane_format_mod_supported() Paulo Zanoni
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Paulo Zanoni @ 2018-09-25 0:19 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
Because we can, the places where we use them already expect const
structs.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fdff1779f778..a21ec8ae46f6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13467,7 +13467,7 @@ static bool intel_cursor_format_mod_supported(struct drm_plane *_plane,
format == DRM_FORMAT_ARGB8888;
}
-static struct drm_plane_funcs skl_plane_funcs = {
+static const struct drm_plane_funcs skl_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
.destroy = intel_plane_destroy,
@@ -13478,7 +13478,7 @@ static struct drm_plane_funcs skl_plane_funcs = {
.format_mod_supported = skl_plane_format_mod_supported,
};
-static struct drm_plane_funcs i965_plane_funcs = {
+static const struct drm_plane_funcs i965_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
.destroy = intel_plane_destroy,
@@ -13489,7 +13489,7 @@ static struct drm_plane_funcs i965_plane_funcs = {
.format_mod_supported = i965_plane_format_mod_supported,
};
-static struct drm_plane_funcs i8xx_plane_funcs = {
+static const struct drm_plane_funcs i8xx_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
.destroy = intel_plane_destroy,
--
2.14.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] drm/i915: make the primary plane func structs const
2018-09-25 0:19 ` [PATCH 2/3] drm/i915: make the primary plane func structs const Paulo Zanoni
@ 2018-09-25 12:05 ` Ville Syrjälä
2018-09-25 22:05 ` Paulo Zanoni
0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2018-09-25 12:05 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
On Mon, Sep 24, 2018 at 05:19:12PM -0700, Paulo Zanoni wrote:
> Because we can, the places where we use them already expect const
> structs.
https://patchwork.freedesktop.org/series/44104/ already has the rest of
the series covered. It didn't get pushed due to lack of drm-misc
backmerge. I suppose that's no longer an issue, but now the series
most likely needs a rebase.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fdff1779f778..a21ec8ae46f6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13467,7 +13467,7 @@ static bool intel_cursor_format_mod_supported(struct drm_plane *_plane,
> format == DRM_FORMAT_ARGB8888;
> }
>
> -static struct drm_plane_funcs skl_plane_funcs = {
> +static const struct drm_plane_funcs skl_plane_funcs = {
> .update_plane = drm_atomic_helper_update_plane,
> .disable_plane = drm_atomic_helper_disable_plane,
> .destroy = intel_plane_destroy,
> @@ -13478,7 +13478,7 @@ static struct drm_plane_funcs skl_plane_funcs = {
> .format_mod_supported = skl_plane_format_mod_supported,
> };
>
> -static struct drm_plane_funcs i965_plane_funcs = {
> +static const struct drm_plane_funcs i965_plane_funcs = {
> .update_plane = drm_atomic_helper_update_plane,
> .disable_plane = drm_atomic_helper_disable_plane,
> .destroy = intel_plane_destroy,
> @@ -13489,7 +13489,7 @@ static struct drm_plane_funcs i965_plane_funcs = {
> .format_mod_supported = i965_plane_format_mod_supported,
> };
>
> -static struct drm_plane_funcs i8xx_plane_funcs = {
> +static const struct drm_plane_funcs i8xx_plane_funcs = {
> .update_plane = drm_atomic_helper_update_plane,
> .disable_plane = drm_atomic_helper_disable_plane,
> .destroy = intel_plane_destroy,
> --
> 2.14.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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] 12+ messages in thread
* Re: [PATCH 2/3] drm/i915: make the primary plane func structs const
2018-09-25 12:05 ` Ville Syrjälä
@ 2018-09-25 22:05 ` Paulo Zanoni
0 siblings, 0 replies; 12+ messages in thread
From: Paulo Zanoni @ 2018-09-25 22:05 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Em Ter, 2018-09-25 às 15:05 +0300, Ville Syrjälä escreveu:
> On Mon, Sep 24, 2018 at 05:19:12PM -0700, Paulo Zanoni wrote:
> > Because we can, the places where we use them already expect const
> > structs.
>
> https://patchwork.freedesktop.org/series/44104/ already has the rest
> of
> the series covered. It didn't get pushed due to lack of drm-misc
> backmerge. I suppose that's no longer an issue, but now the series
> most likely needs a rebase.
Please try to move forward with what's already reviewed so we can
prevent a third person from reimplementing these improvements :).
>
> >
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index fdff1779f778..a21ec8ae46f6 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13467,7 +13467,7 @@ static bool
> > intel_cursor_format_mod_supported(struct drm_plane *_plane,
> > format == DRM_FORMAT_ARGB8888;
> > }
> >
> > -static struct drm_plane_funcs skl_plane_funcs = {
> > +static const struct drm_plane_funcs skl_plane_funcs = {
> > .update_plane = drm_atomic_helper_update_plane,
> > .disable_plane = drm_atomic_helper_disable_plane,
> > .destroy = intel_plane_destroy,
> > @@ -13478,7 +13478,7 @@ static struct drm_plane_funcs
> > skl_plane_funcs = {
> > .format_mod_supported = skl_plane_format_mod_supported,
> > };
> >
> > -static struct drm_plane_funcs i965_plane_funcs = {
> > +static const struct drm_plane_funcs i965_plane_funcs = {
> > .update_plane = drm_atomic_helper_update_plane,
> > .disable_plane = drm_atomic_helper_disable_plane,
> > .destroy = intel_plane_destroy,
> > @@ -13489,7 +13489,7 @@ static struct drm_plane_funcs
> > i965_plane_funcs = {
> > .format_mod_supported = i965_plane_format_mod_supported,
> > };
> >
> > -static struct drm_plane_funcs i8xx_plane_funcs = {
> > +static const struct drm_plane_funcs i8xx_plane_funcs = {
> > .update_plane = drm_atomic_helper_update_plane,
> > .disable_plane = drm_atomic_helper_disable_plane,
> > .destroy = intel_plane_destroy,
> > --
> > 2.14.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] drm/i915: remove a copy of skl_plane_format_mod_supported()
2018-09-25 0:19 [PATCH 1/3] drm/i915: DRM_FORMAT_C8 is not possible with Yf tiling Paulo Zanoni
2018-09-25 0:19 ` [PATCH 2/3] drm/i915: make the primary plane func structs const Paulo Zanoni
@ 2018-09-25 0:19 ` Paulo Zanoni
2018-09-25 6:52 ` dhinakaran.pandiyan
2018-09-25 0:57 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: DRM_FORMAT_C8 is not possible with Yf tiling Patchwork
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Paulo Zanoni @ 2018-09-25 0:19 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
A little git-blame'ing suggests that both functions were added by
commit 714244e280de ("drm/i915: Add format modifiers for Intel"), as
skl_mod_supported() on intel_display.c and
skl_plane_format_mod_supported() on intel_sprite.c. At that time they
were different, but right now they are exactly the same, name and tabs
included. Remove one of the copies and make the other non-static.
Given how our hardware has evolved it is unlikely that future
platforms will require a different format_mod_supported() function for
the primary plane vs the sprite plane, and we can always bring back
the additional copy if we need.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 50 ------------------------------------
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_sprite.c | 4 +--
3 files changed, 4 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a21ec8ae46f6..b9a0183dc580 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13410,56 +13410,6 @@ static bool i965_plane_format_mod_supported(struct drm_plane *_plane,
}
}
-static bool skl_plane_format_mod_supported(struct drm_plane *_plane,
- u32 format, u64 modifier)
-{
- struct intel_plane *plane = to_intel_plane(_plane);
-
- switch (modifier) {
- case DRM_FORMAT_MOD_LINEAR:
- case I915_FORMAT_MOD_X_TILED:
- case I915_FORMAT_MOD_Y_TILED:
- case I915_FORMAT_MOD_Yf_TILED:
- break;
- case I915_FORMAT_MOD_Y_TILED_CCS:
- case I915_FORMAT_MOD_Yf_TILED_CCS:
- if (!plane->has_ccs)
- return false;
- break;
- default:
- return false;
- }
-
- switch (format) {
- case DRM_FORMAT_XRGB8888:
- case DRM_FORMAT_XBGR8888:
- case DRM_FORMAT_ARGB8888:
- case DRM_FORMAT_ABGR8888:
- if (is_ccs_modifier(modifier))
- return true;
- /* fall through */
- case DRM_FORMAT_RGB565:
- case DRM_FORMAT_XRGB2101010:
- case DRM_FORMAT_XBGR2101010:
- case DRM_FORMAT_YUYV:
- case DRM_FORMAT_YVYU:
- case DRM_FORMAT_UYVY:
- case DRM_FORMAT_VYUY:
- case DRM_FORMAT_NV12:
- if (modifier == I915_FORMAT_MOD_Yf_TILED)
- return true;
- /* fall through */
- case DRM_FORMAT_C8:
- if (modifier == DRM_FORMAT_MOD_LINEAR ||
- modifier == I915_FORMAT_MOD_X_TILED ||
- modifier == I915_FORMAT_MOD_Y_TILED)
- return true;
- /* fall through */
- default:
- return false;
- }
-}
-
static bool intel_cursor_format_mod_supported(struct drm_plane *_plane,
u32 format, u64 modifier)
{
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bf1c38728a59..313337f03da8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2140,6 +2140,8 @@ 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);
+bool skl_plane_format_mod_supported(struct drm_plane *_plane,
+ u32 format, u64 modifier);
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 8821e59b70ea..db297d257e1e 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1609,8 +1609,8 @@ static bool vlv_sprite_format_mod_supported(struct drm_plane *_plane,
}
}
-static bool skl_plane_format_mod_supported(struct drm_plane *_plane,
- u32 format, u64 modifier)
+bool skl_plane_format_mod_supported(struct drm_plane *_plane,
+ u32 format, u64 modifier)
{
struct intel_plane *plane = to_intel_plane(_plane);
--
2.14.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] drm/i915: remove a copy of skl_plane_format_mod_supported()
2018-09-25 0:19 ` [PATCH 3/3] drm/i915: remove a copy of skl_plane_format_mod_supported() Paulo Zanoni
@ 2018-09-25 6:52 ` dhinakaran.pandiyan
0 siblings, 0 replies; 12+ messages in thread
From: dhinakaran.pandiyan @ 2018-09-25 6:52 UTC (permalink / raw)
To: Paulo Zanoni, intel-gfx, Pandiyan, Dhinakaran, Ville Syrjälä
On Mon, 2018-09-24 at 17:19 -0700, Paulo Zanoni wrote:
> A little git-blame'ing suggests that both functions were added by
> commit 714244e280de ("drm/i915: Add format modifiers for Intel"), as
> skl_mod_supported() on intel_display.c and
> skl_plane_format_mod_supported() on intel_sprite.c. At that time they
> were different, but right now they are exactly the same, name and
> tabs
> included. Remove one of the copies and make the other non-static.
>
> Given how our hardware has evolved it is unlikely that future
> platforms will require a different format_mod_supported() function
> for
> the primary plane vs the sprite plane, and we can always bring
> back
> the additional copy if we need.
Quick review since I knew the function had a duplicate.
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Also noticed that the modifier arrays are exact copies but the pixel
format arrays look different.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 50 --------------------------
> ----------
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_sprite.c | 4 +--
> 3 files changed, 4 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index a21ec8ae46f6..b9a0183dc580 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13410,56 +13410,6 @@ static bool
> i965_plane_format_mod_supported(struct drm_plane *_plane,
> }
> }
>
> -static bool skl_plane_format_mod_supported(struct drm_plane *_plane,
> - u32 format, u64 modifier)
> -{
> - struct intel_plane *plane = to_intel_plane(_plane);
> -
> - switch (modifier) {
> - case DRM_FORMAT_MOD_LINEAR:
> - case I915_FORMAT_MOD_X_TILED:
> - case I915_FORMAT_MOD_Y_TILED:
> - case I915_FORMAT_MOD_Yf_TILED:
> - break;
> - case I915_FORMAT_MOD_Y_TILED_CCS:
> - case I915_FORMAT_MOD_Yf_TILED_CCS:
> - if (!plane->has_ccs)
> - return false;
> - break;
> - default:
> - return false;
> - }
> -
> - switch (format) {
> - case DRM_FORMAT_XRGB8888:
> - case DRM_FORMAT_XBGR8888:
> - case DRM_FORMAT_ARGB8888:
> - case DRM_FORMAT_ABGR8888:
> - if (is_ccs_modifier(modifier))
> - return true;
> - /* fall through */
> - case DRM_FORMAT_RGB565:
> - case DRM_FORMAT_XRGB2101010:
> - case DRM_FORMAT_XBGR2101010:
> - case DRM_FORMAT_YUYV:
> - case DRM_FORMAT_YVYU:
> - case DRM_FORMAT_UYVY:
> - case DRM_FORMAT_VYUY:
> - case DRM_FORMAT_NV12:
> - if (modifier == I915_FORMAT_MOD_Yf_TILED)
> - return true;
> - /* fall through */
> - case DRM_FORMAT_C8:
> - if (modifier == DRM_FORMAT_MOD_LINEAR ||
> - modifier == I915_FORMAT_MOD_X_TILED ||
> - modifier == I915_FORMAT_MOD_Y_TILED)
> - return true;
> - /* fall through */
> - default:
> - return false;
> - }
> -}
> -
> static bool intel_cursor_format_mod_supported(struct drm_plane
> *_plane,
> u32 format, u64
> modifier)
> {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index bf1c38728a59..313337f03da8 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2140,6 +2140,8 @@ 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);
> +bool skl_plane_format_mod_supported(struct drm_plane *_plane,
> + u32 format, u64 modifier);
> 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 8821e59b70ea..db297d257e1e 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1609,8 +1609,8 @@ static bool
> vlv_sprite_format_mod_supported(struct drm_plane *_plane,
> }
> }
>
> -static bool skl_plane_format_mod_supported(struct drm_plane *_plane,
> - u32 format, u64 modifier)
> +bool skl_plane_format_mod_supported(struct drm_plane *_plane,
> + u32 format, u64 modifier)
> {
> struct intel_plane *plane = to_intel_plane(_plane);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: DRM_FORMAT_C8 is not possible with Yf tiling
2018-09-25 0:19 [PATCH 1/3] drm/i915: DRM_FORMAT_C8 is not possible with Yf tiling Paulo Zanoni
2018-09-25 0:19 ` [PATCH 2/3] drm/i915: make the primary plane func structs const Paulo Zanoni
2018-09-25 0:19 ` [PATCH 3/3] drm/i915: remove a copy of skl_plane_format_mod_supported() Paulo Zanoni
@ 2018-09-25 0:57 ` Patchwork
2018-09-25 2:10 ` ✓ Fi.CI.IGT: " Patchwork
2018-09-25 12:02 ` [PATCH 1/3] " Ville Syrjälä
4 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-09-25 0:57 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/3] drm/i915: DRM_FORMAT_C8 is not possible with Yf tiling
URL : https://patchwork.freedesktop.org/series/50115/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4869 -> Patchwork_10268 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/50115/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_10268 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_exec_suspend@basic-s4-devices:
fi-bdw-samus: PASS -> INCOMPLETE (fdo#107773)
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
fi-byt-clapper: PASS -> FAIL (fdo#103191, fdo#107362)
==== Possible fixes ====
igt@drv_selftest@live_hangcheck:
fi-kbl-7560u: INCOMPLETE (fdo#108044) -> PASS
igt@kms_frontbuffer_tracking@basic:
fi-hsw-peppy: DMESG-WARN (fdo#102614) -> PASS
igt@pm_rpm@module-reload:
fi-skl-caroline: INCOMPLETE (fdo#107807) -> PASS
fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
fdo#108044 https://bugs.freedesktop.org/show_bug.cgi?id=108044
== Participating hosts (46 -> 42) ==
Additional (1): fi-skl-guc
Missing (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4869 -> Patchwork_10268
CI_DRM_4869: 9a74a6db272a007c3db063ae3375fbee60a7bd53 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4649: 19b0c74d20d9b53d4c82be14af0909a3b6846010 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10268: 7ea4513bb3c512c1045ed3e253166d213f2172db @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
7ea4513bb3c5 drm/i915: remove a copy of skl_plane_format_mod_supported()
03ad98ef111e drm/i915: make the primary plane func structs const
f1df62edc741 drm/i915: DRM_FORMAT_C8 is not possible with Yf tiling
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10268/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [1/3] drm/i915: DRM_FORMAT_C8 is not possible with Yf tiling
2018-09-25 0:19 [PATCH 1/3] drm/i915: DRM_FORMAT_C8 is not possible with Yf tiling Paulo Zanoni
` (2 preceding siblings ...)
2018-09-25 0:57 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: DRM_FORMAT_C8 is not possible with Yf tiling Patchwork
@ 2018-09-25 2:10 ` Patchwork
2018-09-25 12:02 ` [PATCH 1/3] " Ville Syrjälä
4 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-09-25 2:10 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/3] drm/i915: DRM_FORMAT_C8 is not possible with Yf tiling
URL : https://patchwork.freedesktop.org/series/50115/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4869_full -> Patchwork_10268_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_10268_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_10268_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_10268_full:
=== IGT changes ===
==== Warnings ====
igt@pm_rc6_residency@rc6-accuracy:
shard-kbl: SKIP -> PASS
shard-snb: SKIP -> PASS
== Known issues ==
Here are the changes found in Patchwork_10268_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
shard-snb: NOTRUN -> DMESG-WARN (fdo#107956)
igt@kms_setmode@basic:
shard-snb: NOTRUN -> FAIL (fdo#99912)
==== Possible fixes ====
igt@gem_exec_schedule@smoketest-render:
shard-snb: INCOMPLETE (fdo#105411) -> SKIP
igt@kms_atomic_transition@1x-modeset-transitions-nonblocking:
shard-apl: DMESG-WARN (fdo#105602, fdo#103558) -> PASS +12
fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
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_4869 -> Patchwork_10268
CI_DRM_4869: 9a74a6db272a007c3db063ae3375fbee60a7bd53 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4649: 19b0c74d20d9b53d4c82be14af0909a3b6846010 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10268: 7ea4513bb3c512c1045ed3e253166d213f2172db @ 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_10268/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] drm/i915: DRM_FORMAT_C8 is not possible with Yf tiling
2018-09-25 0:19 [PATCH 1/3] drm/i915: DRM_FORMAT_C8 is not possible with Yf tiling Paulo Zanoni
` (3 preceding siblings ...)
2018-09-25 2:10 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-09-25 12:02 ` Ville Syrjälä
2018-09-25 22:02 ` Paulo Zanoni
4 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2018-09-25 12:02 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
On Mon, Sep 24, 2018 at 05:19:11PM -0700, Paulo Zanoni wrote:
> Function intel_framebuffer_init() checks for the possibilities during
> framebuffer creation (addfb ioctl time). It is missing the fact that
> the indexed format is not supported with Yf tiling.
>
> It is worth noticing that skl_plane_format_mod_supported() correctly
> handles for the C8/Yf combination, but this function runs during
> modeset time, so we only reject the combination later.
>
> Ville recently proposed a new IGT test that only uses addfb to assert
> supported formats, so that IGT was failing. Add the check so we get
> green squares right from the start after Ville merges his test.
I have two of three (possibly) nicer ways to solve this:
https://patchwork.freedesktop.org/series/39700/
https://patchwork.freedesktop.org/series/39383/
https://patchwork.freedesktop.org/series/39813/
Would be nice if someone could figure out a solution (one of those or
perhaps some other solution I didn't think of) that enough people are
willing to accept.
>
> Also drive-by fix the missing /* fall through */ in the chunk we
> modified by just turning it into a "break;" since IMHO breaks are
> easier to read than fall-throughs.
>
> BSpec: 18565
> Testcase: igt/kms_addfb_basic/expected-formats (not merged yet)
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index eb25037d7b38..fdff1779f778 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14473,13 +14473,19 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
> goto err;
> }
> /* fall through */
> - case I915_FORMAT_MOD_Y_TILED:
> case I915_FORMAT_MOD_Yf_TILED:
> + if (mode_cmd->pixel_format == DRM_FORMAT_C8) {
> + DRM_DEBUG_KMS("Indexed format does not support Yf tiling\n");
> + goto err;
> + }
> + /* fall through */
> + case I915_FORMAT_MOD_Y_TILED:
> if (INTEL_GEN(dev_priv) < 9) {
> DRM_DEBUG_KMS("Unsupported tiling 0x%llx!\n",
> mode_cmd->modifier[0]);
> goto err;
> }
> + break;
> case DRM_FORMAT_MOD_LINEAR:
> case I915_FORMAT_MOD_X_TILED:
> break;
> --
> 2.14.4
--
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] 12+ messages in thread
* Re: [PATCH 1/3] drm/i915: DRM_FORMAT_C8 is not possible with Yf tiling
2018-09-25 12:02 ` [PATCH 1/3] " Ville Syrjälä
@ 2018-09-25 22:02 ` Paulo Zanoni
2018-09-27 14:16 ` Ville Syrjälä
0 siblings, 1 reply; 12+ messages in thread
From: Paulo Zanoni @ 2018-09-25 22:02 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Em Ter, 2018-09-25 às 15:02 +0300, Ville Syrjälä escreveu:
> On Mon, Sep 24, 2018 at 05:19:11PM -0700, Paulo Zanoni wrote:
> > Function intel_framebuffer_init() checks for the possibilities
> > during
> > framebuffer creation (addfb ioctl time). It is missing the fact
> > that
> > the indexed format is not supported with Yf tiling.
> >
> > It is worth noticing that skl_plane_format_mod_supported()
> > correctly
> > handles for the C8/Yf combination, but this function runs during
> > modeset time, so we only reject the combination later.
> >
> > Ville recently proposed a new IGT test that only uses addfb to
> > assert
> > supported formats, so that IGT was failing. Add the check so we get
> > green squares right from the start after Ville merges his test.
>
> I have two of three (possibly) nicer ways to solve this:
> https://patchwork.freedesktop.org/series/39700/
I thought about implementing this one when looking at the code. I agree
the duplicated checks are horrible. I thought maybe this model wouldn't
be acceptable due to the inefficiency of always looping over everything
vs the current linear solution.
I see no review comments on this series besides the vc4 patch. Did you
get anything that's not appearing on patchwork?
> https://patchwork.freedesktop.org/series/39383/
You have blocked your own patch with your own review here.
> https://patchwork.freedesktop.org/series/39813/
Looks like there's some potential controversy to be untangled here if
we wish to follow this route.
> solution 4
I guess it would be to simply not have the checks at all. But this
would be an interface change instead of just refactoring code
duplication.
>
> Would be nice if someone could figure out a solution (one of those or
> perhaps some other solution I didn't think of) that enough people are
> willing to accept.
I could see solution 1 moving forward more easily, and even could
volunteer myself to review a rebased version.
In the meantime, we could actually review/commit this immediate fix and
have a correct-but-not-yet-reworked codebase instead of waiting for a
patch that has been abandoned since March. I don't think one series
should block the other.
>
> >
> > Also drive-by fix the missing /* fall through */ in the chunk we
> > modified by just turning it into a "break;" since IMHO breaks are
> > easier to read than fall-throughs.
> >
> > BSpec: 18565
> > Testcase: igt/kms_addfb_basic/expected-formats (not merged yet)
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index eb25037d7b38..fdff1779f778 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14473,13 +14473,19 @@ static int intel_framebuffer_init(struct
> > intel_framebuffer *intel_fb,
> > goto err;
> > }
> > /* fall through */
> > - case I915_FORMAT_MOD_Y_TILED:
> > case I915_FORMAT_MOD_Yf_TILED:
> > + if (mode_cmd->pixel_format == DRM_FORMAT_C8) {
> > + DRM_DEBUG_KMS("Indexed format does not
> > support Yf tiling\n");
> > + goto err;
> > + }
> > + /* fall through */
> > + case I915_FORMAT_MOD_Y_TILED:
> > if (INTEL_GEN(dev_priv) < 9) {
> > DRM_DEBUG_KMS("Unsupported tiling
> > 0x%llx!\n",
> > mode_cmd->modifier[0]);
> > goto err;
> > }
> > + break;
> > case DRM_FORMAT_MOD_LINEAR:
> > case I915_FORMAT_MOD_X_TILED:
> > break;
> > --
> > 2.14.4
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] drm/i915: DRM_FORMAT_C8 is not possible with Yf tiling
2018-09-25 22:02 ` Paulo Zanoni
@ 2018-09-27 14:16 ` Ville Syrjälä
2018-09-27 19:45 ` Paulo Zanoni
0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2018-09-27 14:16 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
On Tue, Sep 25, 2018 at 03:02:21PM -0700, Paulo Zanoni wrote:
> Em Ter, 2018-09-25 às 15:02 +0300, Ville Syrjälä escreveu:
> > On Mon, Sep 24, 2018 at 05:19:11PM -0700, Paulo Zanoni wrote:
> > > Function intel_framebuffer_init() checks for the possibilities
> > > during
> > > framebuffer creation (addfb ioctl time). It is missing the fact
> > > that
> > > the indexed format is not supported with Yf tiling.
> > >
> > > It is worth noticing that skl_plane_format_mod_supported()
> > > correctly
> > > handles for the C8/Yf combination, but this function runs during
> > > modeset time, so we only reject the combination later.
> > >
> > > Ville recently proposed a new IGT test that only uses addfb to
> > > assert
> > > supported formats, so that IGT was failing. Add the check so we get
> > > green squares right from the start after Ville merges his test.
> >
> > I have two of three (possibly) nicer ways to solve this:
> > https://patchwork.freedesktop.org/series/39700/
>
> I thought about implementing this one when looking at the code. I agree
> the duplicated checks are horrible. I thought maybe this model wouldn't
> be acceptable due to the inefficiency of always looping over everything
> vs the current linear solution.
>
> I see no review comments on this series besides the vc4 patch. Did you
> get anything that's not appearing on patchwork?
>
> > https://patchwork.freedesktop.org/series/39383/
>
> You have blocked your own patch with your own review here.
>
> > https://patchwork.freedesktop.org/series/39813/
>
> Looks like there's some potential controversy to be untangled here if
> we wish to follow this route.
>
> > solution 4
>
> I guess it would be to simply not have the checks at all. But this
> would be an interface change instead of just refactoring code
> duplication.
>
>
> >
> > Would be nice if someone could figure out a solution (one of those or
> > perhaps some other solution I didn't think of) that enough people are
> > willing to accept.
>
> I could see solution 1 moving forward more easily, and even could
> volunteer myself to review a rebased version.
>
> In the meantime, we could actually review/commit this immediate fix and
> have a correct-but-not-yet-reworked codebase instead of waiting for a
> patch that has been abandoned since March. I don't think one series
> should block the other.
Sure. Was just trying to trick someone into finishing what I started ;)
Patch is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>
> >
> > >
> > > Also drive-by fix the missing /* fall through */ in the chunk we
> > > modified by just turning it into a "break;" since IMHO breaks are
> > > easier to read than fall-throughs.
> > >
> > > BSpec: 18565
> > > Testcase: igt/kms_addfb_basic/expected-formats (not merged yet)
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 8 +++++++-
> > > 1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index eb25037d7b38..fdff1779f778 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -14473,13 +14473,19 @@ static int intel_framebuffer_init(struct
> > > intel_framebuffer *intel_fb,
> > > goto err;
> > > }
> > > /* fall through */
> > > - case I915_FORMAT_MOD_Y_TILED:
> > > case I915_FORMAT_MOD_Yf_TILED:
> > > + if (mode_cmd->pixel_format == DRM_FORMAT_C8) {
> > > + DRM_DEBUG_KMS("Indexed format does not
> > > support Yf tiling\n");
> > > + goto err;
> > > + }
> > > + /* fall through */
> > > + case I915_FORMAT_MOD_Y_TILED:
> > > if (INTEL_GEN(dev_priv) < 9) {
> > > DRM_DEBUG_KMS("Unsupported tiling
> > > 0x%llx!\n",
> > > mode_cmd->modifier[0]);
> > > goto err;
> > > }
> > > + break;
> > > case DRM_FORMAT_MOD_LINEAR:
> > > case I915_FORMAT_MOD_X_TILED:
> > > break;
> > > --
> > > 2.14.4
> >
> >
--
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] 12+ messages in thread
* Re: [PATCH 1/3] drm/i915: DRM_FORMAT_C8 is not possible with Yf tiling
2018-09-27 14:16 ` Ville Syrjälä
@ 2018-09-27 19:45 ` Paulo Zanoni
0 siblings, 0 replies; 12+ messages in thread
From: Paulo Zanoni @ 2018-09-27 19:45 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Em Qui, 2018-09-27 às 17:16 +0300, Ville Syrjälä escreveu:
> On Tue, Sep 25, 2018 at 03:02:21PM -0700, Paulo Zanoni wrote:
> > Em Ter, 2018-09-25 às 15:02 +0300, Ville Syrjälä escreveu:
> > > On Mon, Sep 24, 2018 at 05:19:11PM -0700, Paulo Zanoni wrote:
> > > > Function intel_framebuffer_init() checks for the possibilities
> > > > during
> > > > framebuffer creation (addfb ioctl time). It is missing the fact
> > > > that
> > > > the indexed format is not supported with Yf tiling.
> > > >
> > > > It is worth noticing that skl_plane_format_mod_supported()
> > > > correctly
> > > > handles for the C8/Yf combination, but this function runs
> > > > during
> > > > modeset time, so we only reject the combination later.
> > > >
> > > > Ville recently proposed a new IGT test that only uses addfb to
> > > > assert
> > > > supported formats, so that IGT was failing. Add the check so we
> > > > get
> > > > green squares right from the start after Ville merges his test.
> > >
> > > I have two of three (possibly) nicer ways to solve this:
> > > https://patchwork.freedesktop.org/series/39700/
> >
> > I thought about implementing this one when looking at the code. I
> > agree
> > the duplicated checks are horrible. I thought maybe this model
> > wouldn't
> > be acceptable due to the inefficiency of always looping over
> > everything
> > vs the current linear solution.
> >
> > I see no review comments on this series besides the vc4 patch. Did
> > you
> > get anything that's not appearing on patchwork?
> >
> > > https://patchwork.freedesktop.org/series/39383/
> >
> > You have blocked your own patch with your own review here.
> >
> > > https://patchwork.freedesktop.org/series/39813/
> >
> > Looks like there's some potential controversy to be untangled here
> > if
> > we wish to follow this route.
> >
> > > solution 4
> >
> > I guess it would be to simply not have the checks at all. But this
> > would be an interface change instead of just refactoring code
> > duplication.
> >
> >
> > >
> > > Would be nice if someone could figure out a solution (one of
> > > those or
> > > perhaps some other solution I didn't think of) that enough people
> > > are
> > > willing to accept.
> >
> > I could see solution 1 moving forward more easily, and even could
> > volunteer myself to review a rebased version.
> >
> > In the meantime, we could actually review/commit this immediate fix
> > and
> > have a correct-but-not-yet-reworked codebase instead of waiting for
> > a
> > patch that has been abandoned since March. I don't think one series
> > should block the other.
>
> Sure. Was just trying to trick someone into finishing what I started
> ;)
>
> Patch is
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Thanks a lot! Feel free to CC me on the series that kills the whole
thing, I agree it's a good thing to do.
>
> >
> >
> > >
> > > >
> > > > Also drive-by fix the missing /* fall through */ in the chunk
> > > > we
> > > > modified by just turning it into a "break;" since IMHO breaks
> > > > are
> > > > easier to read than fall-throughs.
> > > >
> > > > BSpec: 18565
> > > > Testcase: igt/kms_addfb_basic/expected-formats (not merged yet)
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/intel_display.c | 8 +++++++-
> > > > 1 file changed, 7 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > index eb25037d7b38..fdff1779f778 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -14473,13 +14473,19 @@ static int
> > > > intel_framebuffer_init(struct
> > > > intel_framebuffer *intel_fb,
> > > > goto err;
> > > > }
> > > > /* fall through */
> > > > - case I915_FORMAT_MOD_Y_TILED:
> > > > case I915_FORMAT_MOD_Yf_TILED:
> > > > + if (mode_cmd->pixel_format == DRM_FORMAT_C8) {
> > > > + DRM_DEBUG_KMS("Indexed format does not
> > > > support Yf tiling\n");
> > > > + goto err;
> > > > + }
> > > > + /* fall through */
> > > > + case I915_FORMAT_MOD_Y_TILED:
> > > > if (INTEL_GEN(dev_priv) < 9) {
> > > > DRM_DEBUG_KMS("Unsupported tiling
> > > > 0x%llx!\n",
> > > > mode_cmd->modifier[0]);
> > > > goto err;
> > > > }
> > > > + break;
> > > > case DRM_FORMAT_MOD_LINEAR:
> > > > case I915_FORMAT_MOD_X_TILED:
> > > > break;
> > > > --
> > > > 2.14.4
> > >
> > >
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread