* [PATCH 01/37] drm/i915: Add local 'fb' variables
2016-11-18 19:52 [PATCH v2 00/37] drm: Deduplicate fb format information (v2) ville.syrjala
@ 2016-11-18 19:52 ` ville.syrjala
2016-11-30 14:44 ` Daniel Vetter
2016-11-18 19:52 ` [PATCH 17/37] drm/i915: Set fb->dev early on for inherited fbs ville.syrjala
` (8 subsequent siblings)
9 siblings, 1 reply; 34+ messages in thread
From: ville.syrjala @ 2016-11-18 19:52 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Laurent Pinchart
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Add a local 'fb' variable to a few places to get rid of the
'crtc->primary->fb' stuff. Looks neater and helps me with my poor
coccinelle skills later.
While at it switch over to using the pixel format rather than
depth+bpp.
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_overlay.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index fd0e4dac7cc1..ce3667c18e18 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -658,6 +658,8 @@ static bool update_scaling_factors(struct intel_overlay *overlay,
static void update_colorkey(struct intel_overlay *overlay,
struct overlay_registers __iomem *regs)
{
+ const struct drm_framebuffer *fb =
+ overlay->crtc->base.primary->fb;
u32 key = overlay->color_key;
u32 flags;
@@ -665,24 +667,20 @@ static void update_colorkey(struct intel_overlay *overlay,
if (overlay->color_key_enabled)
flags |= DST_KEY_ENABLE;
- switch (overlay->crtc->base.primary->fb->bits_per_pixel) {
- case 8:
+ switch (fb->pixel_format) {
+ case DRM_FORMAT_C8:
key = 0;
flags |= CLK_RGB8I_MASK;
break;
-
- case 16:
- if (overlay->crtc->base.primary->fb->depth == 15) {
- key = RGB15_TO_COLORKEY(key);
- flags |= CLK_RGB15_MASK;
- } else {
- key = RGB16_TO_COLORKEY(key);
- flags |= CLK_RGB16_MASK;
- }
+ case DRM_FORMAT_XRGB1555:
+ key = RGB15_TO_COLORKEY(key);
+ flags |= CLK_RGB15_MASK;
break;
-
- case 24:
- case 32:
+ case DRM_FORMAT_RGB565:
+ key = RGB16_TO_COLORKEY(key);
+ flags |= CLK_RGB16_MASK;
+ break;
+ default:
flags |= CLK_RGB24_MASK;
break;
}
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 01/37] drm/i915: Add local 'fb' variables
2016-11-18 19:52 ` [PATCH 01/37] drm/i915: Add local 'fb' variables ville.syrjala
@ 2016-11-30 14:44 ` Daniel Vetter
0 siblings, 0 replies; 34+ messages in thread
From: Daniel Vetter @ 2016-11-30 14:44 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, Laurent Pinchart, dri-devel
On Fri, Nov 18, 2016 at 09:52:37PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Add a local 'fb' variable to a few places to get rid of the
> 'crtc->primary->fb' stuff. Looks neater and helps me with my poor
> coccinelle skills later.
>
> While at it switch over to using the pixel format rather than
> depth+bpp.
>
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/intel_overlay.c | 26 ++++++++++++--------------
> 1 file changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
> index fd0e4dac7cc1..ce3667c18e18 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -658,6 +658,8 @@ static bool update_scaling_factors(struct intel_overlay *overlay,
> static void update_colorkey(struct intel_overlay *overlay,
> struct overlay_registers __iomem *regs)
> {
> + const struct drm_framebuffer *fb =
> + overlay->crtc->base.primary->fb;
> u32 key = overlay->color_key;
> u32 flags;
>
> @@ -665,24 +667,20 @@ static void update_colorkey(struct intel_overlay *overlay,
> if (overlay->color_key_enabled)
> flags |= DST_KEY_ENABLE;
>
> - switch (overlay->crtc->base.primary->fb->bits_per_pixel) {
> - case 8:
> + switch (fb->pixel_format) {
> + case DRM_FORMAT_C8:
> key = 0;
> flags |= CLK_RGB8I_MASK;
> break;
> -
> - case 16:
> - if (overlay->crtc->base.primary->fb->depth == 15) {
> - key = RGB15_TO_COLORKEY(key);
> - flags |= CLK_RGB15_MASK;
> - } else {
> - key = RGB16_TO_COLORKEY(key);
> - flags |= CLK_RGB16_MASK;
> - }
> + case DRM_FORMAT_XRGB1555:
> + key = RGB15_TO_COLORKEY(key);
> + flags |= CLK_RGB15_MASK;
> break;
> -
> - case 24:
> - case 32:
> + case DRM_FORMAT_RGB565:
> + key = RGB16_TO_COLORKEY(key);
> + flags |= CLK_RGB16_MASK;
> + break;
> + default:
> flags |= CLK_RGB24_MASK;
> break;
> }
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 17/37] drm/i915: Set fb->dev early on for inherited fbs
2016-11-18 19:52 [PATCH v2 00/37] drm: Deduplicate fb format information (v2) ville.syrjala
2016-11-18 19:52 ` [PATCH 01/37] drm/i915: Add local 'fb' variables ville.syrjala
@ 2016-11-18 19:52 ` ville.syrjala
2016-11-30 15:36 ` Daniel Vetter
2016-11-18 19:52 ` [PATCH 21/37] drm/i915: Populate fb->format early " ville.syrjala
` (7 subsequent siblings)
9 siblings, 1 reply; 34+ messages in thread
From: ville.syrjala @ 2016-11-18 19:52 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Laurent Pinchart
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
We want the fbs inherited from the BIOS to be more or less fully working
prior to actually registering them. This will allow us to just pass the
fb to various helper function instead of having to pass all the
different parameters separately.
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 12af936a402d..74a638c8de61 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8704,6 +8704,8 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
fb = &intel_fb->base;
+ fb->dev = dev;
+
if (INTEL_GEN(dev_priv) >= 4) {
if (val & DISPPLANE_TILED) {
plane_config->tiling = I915_TILING_X;
@@ -9734,6 +9736,8 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
fb = &intel_fb->base;
+ fb->dev = dev;
+
val = I915_READ(PLANE_CTL(pipe, 0));
if (!(val & PLANE_CTL_ENABLE))
goto error;
@@ -9846,6 +9850,8 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc,
fb = &intel_fb->base;
+ fb->dev = dev;
+
if (INTEL_GEN(dev_priv) >= 4) {
if (val & DISPPLANE_TILED) {
plane_config->tiling = I915_TILING_X;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 17/37] drm/i915: Set fb->dev early on for inherited fbs
2016-11-18 19:52 ` [PATCH 17/37] drm/i915: Set fb->dev early on for inherited fbs ville.syrjala
@ 2016-11-30 15:36 ` Daniel Vetter
0 siblings, 0 replies; 34+ messages in thread
From: Daniel Vetter @ 2016-11-30 15:36 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, Laurent Pinchart, dri-devel
On Fri, Nov 18, 2016 at 09:52:53PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We want the fbs inherited from the BIOS to be more or less fully working
> prior to actually registering them. This will allow us to just pass the
> fb to various helper function instead of having to pass all the
> different parameters separately.
>
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
I guess eventually we'll grow a WARN_ON(!fb->dev) in
drm_framebuffer_init() to make sure these don't escape? Anyway:
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/intel_display.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 12af936a402d..74a638c8de61 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8704,6 +8704,8 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
>
> fb = &intel_fb->base;
>
> + fb->dev = dev;
> +
> if (INTEL_GEN(dev_priv) >= 4) {
> if (val & DISPPLANE_TILED) {
> plane_config->tiling = I915_TILING_X;
> @@ -9734,6 +9736,8 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
>
> fb = &intel_fb->base;
>
> + fb->dev = dev;
> +
> val = I915_READ(PLANE_CTL(pipe, 0));
> if (!(val & PLANE_CTL_ENABLE))
> goto error;
> @@ -9846,6 +9850,8 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc,
>
> fb = &intel_fb->base;
>
> + fb->dev = dev;
> +
> if (INTEL_GEN(dev_priv) >= 4) {
> if (val & DISPPLANE_TILED) {
> plane_config->tiling = I915_TILING_X;
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 21/37] drm/i915: Populate fb->format early for inherited fbs
2016-11-18 19:52 [PATCH v2 00/37] drm: Deduplicate fb format information (v2) ville.syrjala
2016-11-18 19:52 ` [PATCH 01/37] drm/i915: Add local 'fb' variables ville.syrjala
2016-11-18 19:52 ` [PATCH 17/37] drm/i915: Set fb->dev early on for inherited fbs ville.syrjala
@ 2016-11-18 19:52 ` ville.syrjala
2016-11-30 15:42 ` Daniel Vetter
2016-11-18 19:53 ` [PATCH 24/37] drm/i915: Eliminate the ugly 'fb?:' constructs from the ilk/skl wm code ville.syrjala
` (6 subsequent siblings)
9 siblings, 1 reply; 34+ messages in thread
From: ville.syrjala @ 2016-11-18 19:52 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Laurent Pinchart
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Make sure the framebuffer format info is available as early as possible
for fbs we inherit from the BIOS. This will allow us to use the fb as
if it was fully formed before we register it.
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 74a638c8de61..c45da6766fff 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8717,6 +8717,7 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
fourcc = i9xx_format_to_fourcc(pixel_format);
fb->pixel_format = fourcc;
fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
+ fb->format = drm_format_info(fourcc);
if (INTEL_GEN(dev_priv) >= 4) {
if (plane_config->tiling)
@@ -9748,6 +9749,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
val & PLANE_CTL_ALPHA_MASK);
fb->pixel_format = fourcc;
fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
+ fb->format = drm_format_info(fourcc);
tiling = val & PLANE_CTL_TILED_MASK;
switch (tiling) {
@@ -9863,6 +9865,7 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc,
fourcc = i9xx_format_to_fourcc(pixel_format);
fb->pixel_format = fourcc;
fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
+ fb->format = drm_format_info(fourcc);
base = I915_READ(DSPSURF(pipe)) & 0xfffff000;
if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 21/37] drm/i915: Populate fb->format early for inherited fbs
2016-11-18 19:52 ` [PATCH 21/37] drm/i915: Populate fb->format early " ville.syrjala
@ 2016-11-30 15:42 ` Daniel Vetter
2016-11-30 15:57 ` Ville Syrjälä
2016-11-30 16:09 ` Daniel Vetter
0 siblings, 2 replies; 34+ messages in thread
From: Daniel Vetter @ 2016-11-30 15:42 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, Laurent Pinchart, dri-devel
On Fri, Nov 18, 2016 at 09:52:57PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Make sure the framebuffer format info is available as early as possible
> for fbs we inherit from the BIOS. This will allow us to use the fb as
> if it was fully formed before we register it.
>
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 74a638c8de61..c45da6766fff 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8717,6 +8717,7 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
> fourcc = i9xx_format_to_fourcc(pixel_format);
> fb->pixel_format = fourcc;
> fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
> + fb->format = drm_format_info(fourcc);
>
> if (INTEL_GEN(dev_priv) >= 4) {
> if (plane_config->tiling)
> @@ -9748,6 +9749,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
> val & PLANE_CTL_ALPHA_MASK);
> fb->pixel_format = fourcc;
> fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
> + fb->format = drm_format_info(fourcc);
>
> tiling = val & PLANE_CTL_TILED_MASK;
> switch (tiling) {
> @@ -9863,6 +9865,7 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc,
> fourcc = i9xx_format_to_fourcc(pixel_format);
> fb->pixel_format = fourcc;
> fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
> + fb->format = drm_format_info(fourcc);
Do we really want to hand-roll this all, or could we somehow reuse
fill_fb_struct? Or will this all go away again?
I'll reserve judgement until the end, but this here looks correct ;-)
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> base = I915_READ(DSPSURF(pipe)) & 0xfffff000;
> if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 21/37] drm/i915: Populate fb->format early for inherited fbs
2016-11-30 15:42 ` Daniel Vetter
@ 2016-11-30 15:57 ` Ville Syrjälä
2016-11-30 16:09 ` Daniel Vetter
1 sibling, 0 replies; 34+ messages in thread
From: Ville Syrjälä @ 2016-11-30 15:57 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, Laurent Pinchart, dri-devel
On Wed, Nov 30, 2016 at 04:42:23PM +0100, Daniel Vetter wrote:
> On Fri, Nov 18, 2016 at 09:52:57PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Make sure the framebuffer format info is available as early as possible
> > for fbs we inherit from the BIOS. This will allow us to use the fb as
> > if it was fully formed before we register it.
> >
> > Cc: intel-gfx@lists.freedesktop.org
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 74a638c8de61..c45da6766fff 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -8717,6 +8717,7 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
> > fourcc = i9xx_format_to_fourcc(pixel_format);
> > fb->pixel_format = fourcc;
> > fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
> > + fb->format = drm_format_info(fourcc);
> >
> > if (INTEL_GEN(dev_priv) >= 4) {
> > if (plane_config->tiling)
> > @@ -9748,6 +9749,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
> > val & PLANE_CTL_ALPHA_MASK);
> > fb->pixel_format = fourcc;
> > fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
> > + fb->format = drm_format_info(fourcc);
> >
> > tiling = val & PLANE_CTL_TILED_MASK;
> > switch (tiling) {
> > @@ -9863,6 +9865,7 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc,
> > fourcc = i9xx_format_to_fourcc(pixel_format);
> > fb->pixel_format = fourcc;
> > fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
> > + fb->format = drm_format_info(fourcc);
>
> Do we really want to hand-roll this all, or could we somehow reuse
> fill_fb_struct? Or will this all go away again?
Yeah, I think we'll want to flip all of this over to using
fill_fb_struct(). Just didn't have the energy to look into that yet,
and the series was already getting too long anyway. So figured I'd save
that for the next round.
>
> I'll reserve judgement until the end, but this here looks correct ;-)
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> >
> > base = I915_READ(DSPSURF(pipe)) & 0xfffff000;
> > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> > --
> > 2.7.4
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 21/37] drm/i915: Populate fb->format early for inherited fbs
2016-11-30 15:42 ` Daniel Vetter
2016-11-30 15:57 ` Ville Syrjälä
@ 2016-11-30 16:09 ` Daniel Vetter
1 sibling, 0 replies; 34+ messages in thread
From: Daniel Vetter @ 2016-11-30 16:09 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, Laurent Pinchart, dri-devel
On Wed, Nov 30, 2016 at 04:42:23PM +0100, Daniel Vetter wrote:
> On Fri, Nov 18, 2016 at 09:52:57PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Make sure the framebuffer format info is available as early as possible
> > for fbs we inherit from the BIOS. This will allow us to use the fb as
> > if it was fully formed before we register it.
> >
> > Cc: intel-gfx@lists.freedesktop.org
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 74a638c8de61..c45da6766fff 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -8717,6 +8717,7 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
> > fourcc = i9xx_format_to_fourcc(pixel_format);
> > fb->pixel_format = fourcc;
> > fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
> > + fb->format = drm_format_info(fourcc);
> >
> > if (INTEL_GEN(dev_priv) >= 4) {
> > if (plane_config->tiling)
> > @@ -9748,6 +9749,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
> > val & PLANE_CTL_ALPHA_MASK);
> > fb->pixel_format = fourcc;
> > fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
> > + fb->format = drm_format_info(fourcc);
> >
> > tiling = val & PLANE_CTL_TILED_MASK;
> > switch (tiling) {
> > @@ -9863,6 +9865,7 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc,
> > fourcc = i9xx_format_to_fourcc(pixel_format);
> > fb->pixel_format = fourcc;
> > fb->bits_per_pixel = drm_format_plane_cpp(fourcc, 0) * 8;
> > + fb->format = drm_format_info(fourcc);
>
> Do we really want to hand-roll this all, or could we somehow reuse
> fill_fb_struct? Or will this all go away again?
Looking at the end result I think doing the fill_fb_struct a pile earlier
would be nice. Or at least extract an set_fb_format helper or whatever to
set ->format and ->dev.
Needs a better name than set_fb_format though ;-)
Anyway, follow-up work.
-Daniel
>
> I'll reserve judgement until the end, but this here looks correct ;-)
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> >
> > base = I915_READ(DSPSURF(pipe)) & 0xfffff000;
> > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> > --
> > 2.7.4
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 24/37] drm/i915: Eliminate the ugly 'fb?:' constructs from the ilk/skl wm code
2016-11-18 19:52 [PATCH v2 00/37] drm: Deduplicate fb format information (v2) ville.syrjala
` (2 preceding siblings ...)
2016-11-18 19:52 ` [PATCH 21/37] drm/i915: Populate fb->format early " ville.syrjala
@ 2016-11-18 19:53 ` ville.syrjala
2016-11-30 15:51 ` [Intel-gfx] " Daniel Vetter
2016-11-18 19:53 ` [PATCH 28/37] drm/i915: Store a pointer to the pixel format info for fbc ville.syrjala
` (5 subsequent siblings)
9 siblings, 1 reply; 34+ messages in thread
From: ville.syrjala @ 2016-11-18 19:53 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Laurent Pinchart
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Don't access plane_state->fb until we know the plane to be visible.
It it's visible, it will have an fb, and thus we don't have to
consider the NULL fb case. Makes the code look nicer.
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index bbb1eaf1e6db..8ba7413872dd 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1781,13 +1781,14 @@ static uint32_t ilk_compute_pri_wm(const struct intel_crtc_state *cstate,
uint32_t mem_value,
bool is_lp)
{
- int cpp = pstate->base.fb ?
- drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0;
uint32_t method1, method2;
+ int cpp;
if (!cstate->base.active || !pstate->base.visible)
return 0;
+ cpp = drm_format_plane_cpp(pstate->base.fb->pixel_format, 0);
+
method1 = ilk_wm_method1(ilk_pipe_pixel_rate(cstate), cpp, mem_value);
if (!is_lp)
@@ -1809,13 +1810,14 @@ static uint32_t ilk_compute_spr_wm(const struct intel_crtc_state *cstate,
const struct intel_plane_state *pstate,
uint32_t mem_value)
{
- int cpp = pstate->base.fb ?
- drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0;
uint32_t method1, method2;
+ int cpp;
if (!cstate->base.active || !pstate->base.visible)
return 0;
+ cpp = drm_format_plane_cpp(pstate->base.fb->pixel_format, 0);
+
method1 = ilk_wm_method1(ilk_pipe_pixel_rate(cstate), cpp, mem_value);
method2 = ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
cstate->base.adjusted_mode.crtc_htotal,
@@ -1853,12 +1855,13 @@ static uint32_t ilk_compute_fbc_wm(const struct intel_crtc_state *cstate,
const struct intel_plane_state *pstate,
uint32_t pri_val)
{
- int cpp = pstate->base.fb ?
- drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0;
+ int cpp;
if (!cstate->base.active || !pstate->base.visible)
return 0;
+ cpp = drm_format_plane_cpp(pstate->base.fb->pixel_format, 0);
+
return ilk_wm_fbc(pri_val, drm_rect_width(&pstate->base.dst), cpp);
}
@@ -3229,13 +3232,17 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
int y)
{
struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
- struct drm_framebuffer *fb = pstate->fb;
uint32_t down_scale_amount, data_rate;
uint32_t width = 0, height = 0;
- unsigned format = fb ? fb->pixel_format : DRM_FORMAT_XRGB8888;
+ struct drm_framebuffer *fb;
+ u32 format;
if (!intel_pstate->base.visible)
return 0;
+
+ fb = pstate->fb;
+ format = fb->pixel_format;
+
if (pstate->plane->type == DRM_PLANE_TYPE_CURSOR)
return 0;
if (y && format != DRM_FORMAT_NV12)
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Intel-gfx] [PATCH 24/37] drm/i915: Eliminate the ugly 'fb?:' constructs from the ilk/skl wm code
2016-11-18 19:53 ` [PATCH 24/37] drm/i915: Eliminate the ugly 'fb?:' constructs from the ilk/skl wm code ville.syrjala
@ 2016-11-30 15:51 ` Daniel Vetter
2016-11-30 15:59 ` Ville Syrjälä
0 siblings, 1 reply; 34+ messages in thread
From: Daniel Vetter @ 2016-11-30 15:51 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, Laurent Pinchart, dri-devel
On Fri, Nov 18, 2016 at 09:53:00PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Don't access plane_state->fb until we know the plane to be visible.
> It it's visible, it will have an fb, and thus we don't have to
> consider the NULL fb case. Makes the code look nicer.
>
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index bbb1eaf1e6db..8ba7413872dd 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1781,13 +1781,14 @@ static uint32_t ilk_compute_pri_wm(const struct intel_crtc_state *cstate,
> uint32_t mem_value,
> bool is_lp)
> {
> - int cpp = pstate->base.fb ?
> - drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0;
> uint32_t method1, method2;
> + int cpp;
>
> if (!cstate->base.active || !pstate->base.visible)
> return 0;
Why do we still look for crtc_state->active here? Sounds like a bug in
proper validating our wm needs. Anway, change itself looks good.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> + cpp = drm_format_plane_cpp(pstate->base.fb->pixel_format, 0);
> +
> method1 = ilk_wm_method1(ilk_pipe_pixel_rate(cstate), cpp, mem_value);
>
> if (!is_lp)
> @@ -1809,13 +1810,14 @@ static uint32_t ilk_compute_spr_wm(const struct intel_crtc_state *cstate,
> const struct intel_plane_state *pstate,
> uint32_t mem_value)
> {
> - int cpp = pstate->base.fb ?
> - drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0;
> uint32_t method1, method2;
> + int cpp;
>
> if (!cstate->base.active || !pstate->base.visible)
> return 0;
>
> + cpp = drm_format_plane_cpp(pstate->base.fb->pixel_format, 0);
> +
> method1 = ilk_wm_method1(ilk_pipe_pixel_rate(cstate), cpp, mem_value);
> method2 = ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
> cstate->base.adjusted_mode.crtc_htotal,
> @@ -1853,12 +1855,13 @@ static uint32_t ilk_compute_fbc_wm(const struct intel_crtc_state *cstate,
> const struct intel_plane_state *pstate,
> uint32_t pri_val)
> {
> - int cpp = pstate->base.fb ?
> - drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0;
> + int cpp;
>
> if (!cstate->base.active || !pstate->base.visible)
> return 0;
>
> + cpp = drm_format_plane_cpp(pstate->base.fb->pixel_format, 0);
> +
> return ilk_wm_fbc(pri_val, drm_rect_width(&pstate->base.dst), cpp);
> }
>
> @@ -3229,13 +3232,17 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
> int y)
> {
> struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
> - struct drm_framebuffer *fb = pstate->fb;
> uint32_t down_scale_amount, data_rate;
> uint32_t width = 0, height = 0;
> - unsigned format = fb ? fb->pixel_format : DRM_FORMAT_XRGB8888;
> + struct drm_framebuffer *fb;
> + u32 format;
>
> if (!intel_pstate->base.visible)
> return 0;
> +
> + fb = pstate->fb;
> + format = fb->pixel_format;
> +
> if (pstate->plane->type == DRM_PLANE_TYPE_CURSOR)
> return 0;
> if (y && format != DRM_FORMAT_NV12)
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [Intel-gfx] [PATCH 24/37] drm/i915: Eliminate the ugly 'fb?:' constructs from the ilk/skl wm code
2016-11-30 15:51 ` [Intel-gfx] " Daniel Vetter
@ 2016-11-30 15:59 ` Ville Syrjälä
0 siblings, 0 replies; 34+ messages in thread
From: Ville Syrjälä @ 2016-11-30 15:59 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, Laurent Pinchart, dri-devel
On Wed, Nov 30, 2016 at 04:51:33PM +0100, Daniel Vetter wrote:
> On Fri, Nov 18, 2016 at 09:53:00PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Don't access plane_state->fb until we know the plane to be visible.
> > It it's visible, it will have an fb, and thus we don't have to
> > consider the NULL fb case. Makes the code look nicer.
> >
> > Cc: intel-gfx@lists.freedesktop.org
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_pm.c | 23 +++++++++++++++--------
> > 1 file changed, 15 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index bbb1eaf1e6db..8ba7413872dd 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -1781,13 +1781,14 @@ static uint32_t ilk_compute_pri_wm(const struct intel_crtc_state *cstate,
> > uint32_t mem_value,
> > bool is_lp)
> > {
> > - int cpp = pstate->base.fb ?
> > - drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0;
> > uint32_t method1, method2;
> > + int cpp;
> >
> > if (!cstate->base.active || !pstate->base.visible)
> > return 0;
>
> Why do we still look for crtc_state->active here? Sounds like a bug in
> proper validating our wm needs.
Yeah, unfortunately that thing is still broken all over :( There are
broken assumptions about this higher up as well, so fixing this will
involve actual work I fear.
> Anway, change itself looks good.
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> >
> > + cpp = drm_format_plane_cpp(pstate->base.fb->pixel_format, 0);
> > +
> > method1 = ilk_wm_method1(ilk_pipe_pixel_rate(cstate), cpp, mem_value);
> >
> > if (!is_lp)
> > @@ -1809,13 +1810,14 @@ static uint32_t ilk_compute_spr_wm(const struct intel_crtc_state *cstate,
> > const struct intel_plane_state *pstate,
> > uint32_t mem_value)
> > {
> > - int cpp = pstate->base.fb ?
> > - drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0;
> > uint32_t method1, method2;
> > + int cpp;
> >
> > if (!cstate->base.active || !pstate->base.visible)
> > return 0;
> >
> > + cpp = drm_format_plane_cpp(pstate->base.fb->pixel_format, 0);
> > +
> > method1 = ilk_wm_method1(ilk_pipe_pixel_rate(cstate), cpp, mem_value);
> > method2 = ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
> > cstate->base.adjusted_mode.crtc_htotal,
> > @@ -1853,12 +1855,13 @@ static uint32_t ilk_compute_fbc_wm(const struct intel_crtc_state *cstate,
> > const struct intel_plane_state *pstate,
> > uint32_t pri_val)
> > {
> > - int cpp = pstate->base.fb ?
> > - drm_format_plane_cpp(pstate->base.fb->pixel_format, 0) : 0;
> > + int cpp;
> >
> > if (!cstate->base.active || !pstate->base.visible)
> > return 0;
> >
> > + cpp = drm_format_plane_cpp(pstate->base.fb->pixel_format, 0);
> > +
> > return ilk_wm_fbc(pri_val, drm_rect_width(&pstate->base.dst), cpp);
> > }
> >
> > @@ -3229,13 +3232,17 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
> > int y)
> > {
> > struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
> > - struct drm_framebuffer *fb = pstate->fb;
> > uint32_t down_scale_amount, data_rate;
> > uint32_t width = 0, height = 0;
> > - unsigned format = fb ? fb->pixel_format : DRM_FORMAT_XRGB8888;
> > + struct drm_framebuffer *fb;
> > + u32 format;
> >
> > if (!intel_pstate->base.visible)
> > return 0;
> > +
> > + fb = pstate->fb;
> > + format = fb->pixel_format;
> > +
> > if (pstate->plane->type == DRM_PLANE_TYPE_CURSOR)
> > return 0;
> > if (y && format != DRM_FORMAT_NV12)
> > --
> > 2.7.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 28/37] drm/i915: Store a pointer to the pixel format info for fbc
2016-11-18 19:52 [PATCH v2 00/37] drm: Deduplicate fb format information (v2) ville.syrjala
` (3 preceding siblings ...)
2016-11-18 19:53 ` [PATCH 24/37] drm/i915: Eliminate the ugly 'fb?:' constructs from the ilk/skl wm code ville.syrjala
@ 2016-11-18 19:53 ` ville.syrjala
2016-11-30 16:07 ` [Intel-gfx] " Daniel Vetter
2016-11-18 19:53 ` [PATCH 30/37] drm/i915: Use drm_framebuffer_plane_{width, height}() where possible ville.syrjala
` (4 subsequent siblings)
9 siblings, 1 reply; 34+ messages in thread
From: ville.syrjala @ 2016-11-18 19:53 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Laurent Pinchart, Paulo Zanoni
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Rather than store the pixel format and look up the format info as
needed, let's just store a pointer to the format info directly
and speed up our lookups.
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 4 ++--
drivers/gpu/drm/i915/intel_fbc.c | 14 +++++++-------
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index be67aeece749..692b79e056be 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1026,7 +1026,7 @@ struct intel_fbc {
struct {
u64 ilk_ggtt_offset;
- uint32_t pixel_format;
+ const struct drm_format_info *format;
unsigned int stride;
int fence_reg;
unsigned int tiling_mode;
@@ -1042,7 +1042,7 @@ struct intel_fbc {
struct {
u64 ggtt_offset;
- uint32_t pixel_format;
+ const struct drm_format_info *format;
unsigned int stride;
int fence_reg;
} fb;
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 62f215b12eb5..659cebc3bfd2 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -188,7 +188,7 @@ static void g4x_fbc_activate(struct drm_i915_private *dev_priv)
u32 dpfc_ctl;
dpfc_ctl = DPFC_CTL_PLANE(params->crtc.plane) | DPFC_SR_EN;
- if (drm_format_plane_cpp(params->fb.pixel_format, 0) == 2)
+ if (params->fb.format->cpp[0] == 2)
dpfc_ctl |= DPFC_CTL_LIMIT_2X;
else
dpfc_ctl |= DPFC_CTL_LIMIT_1X;
@@ -235,7 +235,7 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
int threshold = dev_priv->fbc.threshold;
dpfc_ctl = DPFC_CTL_PLANE(params->crtc.plane);
- if (drm_format_plane_cpp(params->fb.pixel_format, 0) == 2)
+ if (params->fb.format->cpp[0] == 2)
threshold++;
switch (threshold) {
@@ -303,7 +303,7 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
if (IS_IVYBRIDGE(dev_priv))
dpfc_ctl |= IVB_DPFC_CTL_PLANE(params->crtc.plane);
- if (drm_format_plane_cpp(params->fb.pixel_format, 0) == 2)
+ if (params->fb.format->cpp[0] == 2)
threshold++;
switch (threshold) {
@@ -581,7 +581,7 @@ static int intel_fbc_alloc_cfb(struct intel_crtc *crtc)
WARN_ON(drm_mm_node_allocated(&fbc->compressed_fb));
size = intel_fbc_calculate_cfb_size(dev_priv, &fbc->state_cache);
- fb_cpp = drm_format_plane_cpp(fbc->state_cache.fb.pixel_format, 0);
+ fb_cpp = fbc->state_cache.fb.format->cpp[0];
ret = find_compression_threshold(dev_priv, &fbc->compressed_fb,
size, fb_cpp);
@@ -764,7 +764,7 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
* platforms that need. */
if (IS_GEN(dev_priv, 5, 6))
cache->fb.ilk_ggtt_offset = i915_gem_object_ggtt_offset(obj, NULL);
- cache->fb.pixel_format = fb->pixel_format;
+ cache->fb.format = fb->format;
cache->fb.stride = fb->pitches[0];
cache->fb.fence_reg = get_fence_id(fb);
cache->fb.tiling_mode = i915_gem_object_get_tiling(obj);
@@ -823,7 +823,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
return false;
}
- if (!pixel_format_is_valid(dev_priv, cache->fb.pixel_format)) {
+ if (!pixel_format_is_valid(dev_priv, cache->fb.format->format)) {
fbc->no_fbc_reason = "pixel format is invalid";
return false;
}
@@ -892,7 +892,7 @@ static void intel_fbc_get_reg_params(struct intel_crtc *crtc,
params->crtc.plane = crtc->plane;
params->crtc.fence_y_offset = get_crtc_fence_y_offset(crtc);
- params->fb.pixel_format = cache->fb.pixel_format;
+ params->fb.format = cache->fb.format;
params->fb.stride = cache->fb.stride;
params->fb.fence_reg = cache->fb.fence_reg;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Intel-gfx] [PATCH 28/37] drm/i915: Store a pointer to the pixel format info for fbc
2016-11-18 19:53 ` [PATCH 28/37] drm/i915: Store a pointer to the pixel format info for fbc ville.syrjala
@ 2016-11-30 16:07 ` Daniel Vetter
0 siblings, 0 replies; 34+ messages in thread
From: Daniel Vetter @ 2016-11-30 16:07 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, Laurent Pinchart, dri-devel, Paulo Zanoni
On Fri, Nov 18, 2016 at 09:53:04PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Rather than store the pixel format and look up the format info as
> needed, let's just store a pointer to the format info directly
> and speed up our lookups.
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Assuming it all compiles still:
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 4 ++--
> drivers/gpu/drm/i915/intel_fbc.c | 14 +++++++-------
> 2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index be67aeece749..692b79e056be 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1026,7 +1026,7 @@ struct intel_fbc {
>
> struct {
> u64 ilk_ggtt_offset;
> - uint32_t pixel_format;
> + const struct drm_format_info *format;
> unsigned int stride;
> int fence_reg;
> unsigned int tiling_mode;
> @@ -1042,7 +1042,7 @@ struct intel_fbc {
>
> struct {
> u64 ggtt_offset;
> - uint32_t pixel_format;
> + const struct drm_format_info *format;
> unsigned int stride;
> int fence_reg;
> } fb;
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 62f215b12eb5..659cebc3bfd2 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -188,7 +188,7 @@ static void g4x_fbc_activate(struct drm_i915_private *dev_priv)
> u32 dpfc_ctl;
>
> dpfc_ctl = DPFC_CTL_PLANE(params->crtc.plane) | DPFC_SR_EN;
> - if (drm_format_plane_cpp(params->fb.pixel_format, 0) == 2)
> + if (params->fb.format->cpp[0] == 2)
> dpfc_ctl |= DPFC_CTL_LIMIT_2X;
> else
> dpfc_ctl |= DPFC_CTL_LIMIT_1X;
> @@ -235,7 +235,7 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
> int threshold = dev_priv->fbc.threshold;
>
> dpfc_ctl = DPFC_CTL_PLANE(params->crtc.plane);
> - if (drm_format_plane_cpp(params->fb.pixel_format, 0) == 2)
> + if (params->fb.format->cpp[0] == 2)
> threshold++;
>
> switch (threshold) {
> @@ -303,7 +303,7 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
> if (IS_IVYBRIDGE(dev_priv))
> dpfc_ctl |= IVB_DPFC_CTL_PLANE(params->crtc.plane);
>
> - if (drm_format_plane_cpp(params->fb.pixel_format, 0) == 2)
> + if (params->fb.format->cpp[0] == 2)
> threshold++;
>
> switch (threshold) {
> @@ -581,7 +581,7 @@ static int intel_fbc_alloc_cfb(struct intel_crtc *crtc)
> WARN_ON(drm_mm_node_allocated(&fbc->compressed_fb));
>
> size = intel_fbc_calculate_cfb_size(dev_priv, &fbc->state_cache);
> - fb_cpp = drm_format_plane_cpp(fbc->state_cache.fb.pixel_format, 0);
> + fb_cpp = fbc->state_cache.fb.format->cpp[0];
>
> ret = find_compression_threshold(dev_priv, &fbc->compressed_fb,
> size, fb_cpp);
> @@ -764,7 +764,7 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
> * platforms that need. */
> if (IS_GEN(dev_priv, 5, 6))
> cache->fb.ilk_ggtt_offset = i915_gem_object_ggtt_offset(obj, NULL);
> - cache->fb.pixel_format = fb->pixel_format;
> + cache->fb.format = fb->format;
> cache->fb.stride = fb->pitches[0];
> cache->fb.fence_reg = get_fence_id(fb);
> cache->fb.tiling_mode = i915_gem_object_get_tiling(obj);
> @@ -823,7 +823,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
> return false;
> }
>
> - if (!pixel_format_is_valid(dev_priv, cache->fb.pixel_format)) {
> + if (!pixel_format_is_valid(dev_priv, cache->fb.format->format)) {
> fbc->no_fbc_reason = "pixel format is invalid";
> return false;
> }
> @@ -892,7 +892,7 @@ static void intel_fbc_get_reg_params(struct intel_crtc *crtc,
> params->crtc.plane = crtc->plane;
> params->crtc.fence_y_offset = get_crtc_fence_y_offset(crtc);
>
> - params->fb.pixel_format = cache->fb.pixel_format;
> + params->fb.format = cache->fb.format;
> params->fb.stride = cache->fb.stride;
> params->fb.fence_reg = cache->fb.fence_reg;
>
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 30/37] drm/i915: Use drm_framebuffer_plane_{width, height}() where possible
2016-11-18 19:52 [PATCH v2 00/37] drm: Deduplicate fb format information (v2) ville.syrjala
` (4 preceding siblings ...)
2016-11-18 19:53 ` [PATCH 28/37] drm/i915: Store a pointer to the pixel format info for fbc ville.syrjala
@ 2016-11-18 19:53 ` ville.syrjala
2016-11-30 16:04 ` Daniel Vetter
2016-11-30 16:06 ` [Intel-gfx] " Daniel Vetter
2016-11-18 19:53 ` [PATCH 36/37] drm: Add mode_config .get_format_info() hook ville.syrjala
` (3 subsequent siblings)
9 siblings, 2 replies; 34+ messages in thread
From: ville.syrjala @ 2016-11-18 19:53 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Laurent Pinchart
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Replace drm_format_plane_{width,height}() usage with
drm_framebuffer_plane_{width,height}() to avoid the lookup of the format
info.
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8f63fd38deee..5d8db436c557 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2496,7 +2496,6 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv,
struct intel_rotation_info *rot_info = &intel_fb->rot_info;
u32 gtt_offset_rotated = 0;
unsigned int max_size = 0;
- uint32_t format = fb->pixel_format;
int i, num_planes = fb->format->num_planes;
unsigned int tile_size = intel_tile_size(dev_priv);
@@ -2507,8 +2506,8 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv,
int x, y;
cpp = fb->format->cpp[i];
- width = drm_format_plane_width(fb->width, format, i);
- height = drm_format_plane_height(fb->height, format, i);
+ width = drm_framebuffer_plane_width(fb->width, fb, i);
+ height = drm_framebuffer_plane_height(fb->height, fb, i);
intel_fb_offset_to_xy(&x, &y, fb, i);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 30/37] drm/i915: Use drm_framebuffer_plane_{width, height}() where possible
2016-11-18 19:53 ` [PATCH 30/37] drm/i915: Use drm_framebuffer_plane_{width, height}() where possible ville.syrjala
@ 2016-11-30 16:04 ` Daniel Vetter
2016-11-30 16:06 ` [Intel-gfx] " Daniel Vetter
1 sibling, 0 replies; 34+ messages in thread
From: Daniel Vetter @ 2016-11-30 16:04 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, Laurent Pinchart, dri-devel
On Fri, Nov 18, 2016 at 09:53:06PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Replace drm_format_plane_{width,height}() usage with
> drm_framebuffer_plane_{width,height}() to avoid the lookup of the format
> info.
>
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/intel_display.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8f63fd38deee..5d8db436c557 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2496,7 +2496,6 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv,
> struct intel_rotation_info *rot_info = &intel_fb->rot_info;
> u32 gtt_offset_rotated = 0;
> unsigned int max_size = 0;
> - uint32_t format = fb->pixel_format;
> int i, num_planes = fb->format->num_planes;
> unsigned int tile_size = intel_tile_size(dev_priv);
>
> @@ -2507,8 +2506,8 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv,
> int x, y;
>
> cpp = fb->format->cpp[i];
> - width = drm_format_plane_width(fb->width, format, i);
> - height = drm_format_plane_height(fb->height, format, i);
> + width = drm_framebuffer_plane_width(fb->width, fb, i);
> + height = drm_framebuffer_plane_height(fb->height, fb, i);
>
> intel_fb_offset_to_xy(&x, &y, fb, i);
>
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [Intel-gfx] [PATCH 30/37] drm/i915: Use drm_framebuffer_plane_{width, height}() where possible
2016-11-18 19:53 ` [PATCH 30/37] drm/i915: Use drm_framebuffer_plane_{width, height}() where possible ville.syrjala
2016-11-30 16:04 ` Daniel Vetter
@ 2016-11-30 16:06 ` Daniel Vetter
1 sibling, 0 replies; 34+ messages in thread
From: Daniel Vetter @ 2016-11-30 16:06 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, Laurent Pinchart, dri-devel
On Fri, Nov 18, 2016 at 09:53:06PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Replace drm_format_plane_{width,height}() usage with
> drm_framebuffer_plane_{width,height}() to avoid the lookup of the format
> info.
>
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8f63fd38deee..5d8db436c557 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2496,7 +2496,6 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv,
> struct intel_rotation_info *rot_info = &intel_fb->rot_info;
> u32 gtt_offset_rotated = 0;
> unsigned int max_size = 0;
> - uint32_t format = fb->pixel_format;
> int i, num_planes = fb->format->num_planes;
> unsigned int tile_size = intel_tile_size(dev_priv);
>
> @@ -2507,8 +2506,8 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv,
> int x, y;
>
> cpp = fb->format->cpp[i];
> - width = drm_format_plane_width(fb->width, format, i);
> - height = drm_format_plane_height(fb->height, format, i);
Grep seems to say that we can nuke these two after this patch? I didn't
find that patch in your series ...
-Daniel
> + width = drm_framebuffer_plane_width(fb->width, fb, i);
> + height = drm_framebuffer_plane_height(fb->height, fb, i);
>
> intel_fb_offset_to_xy(&x, &y, fb, i);
>
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 36/37] drm: Add mode_config .get_format_info() hook
2016-11-18 19:52 [PATCH v2 00/37] drm: Deduplicate fb format information (v2) ville.syrjala
` (5 preceding siblings ...)
2016-11-18 19:53 ` [PATCH 30/37] drm/i915: Use drm_framebuffer_plane_{width, height}() where possible ville.syrjala
@ 2016-11-18 19:53 ` ville.syrjala
2016-11-20 8:13 ` Laurent Pinchart
2016-11-22 13:41 ` [PATCH v2 " ville.syrjala
2016-11-18 19:53 ` [PATCH 37/37] drm/i915: Implement .get_format_info() hook for CCS ville.syrjala
` (2 subsequent siblings)
9 siblings, 2 replies; 34+ messages in thread
From: ville.syrjala @ 2016-11-18 19:53 UTC (permalink / raw)
To: dri-devel; +Cc: Ben Widawsky, intel-gfx, Laurent Pinchart
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Allow drivers to return a custom drm_format_info structure for special
fb layouts. We'll use this for the compression control surface in i915.
Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_fb_cma_helper.c | 2 +-
drivers/gpu/drm/drm_fourcc.c | 25 +++++++++++++++++++++++++
drivers/gpu/drm/drm_framebuffer.c | 9 +++++++--
drivers/gpu/drm/drm_modeset_helper.c | 2 +-
include/drm/drm_fourcc.h | 6 ++++++
include/drm/drm_mode_config.h | 15 +++++++++++++++
6 files changed, 55 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index aab4465307ed..d7f8876cf5e9 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -186,7 +186,7 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,
int ret;
int i;
- info = drm_format_info(mode_cmd->pixel_format);
+ info = drm_get_format_info(dev, mode_cmd);
if (!info)
return ERR_PTR(-EINVAL);
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 90d2cc8da8eb..7cfaee689f0c 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -199,6 +199,31 @@ const struct drm_format_info *drm_format_info(u32 format)
EXPORT_SYMBOL(drm_format_info);
/**
+ * drm_format_info - query information for a given framebuffer configuration
+ * @dev: DRM device
+ * @mode_cmd: metadata from the userspace fb creation request
+ *
+ * Returns:
+ * The instance of struct drm_format_info that describes the pixel format, or
+ * NULL if the format is unsupported.
+ */
+const struct drm_format_info *
+drm_get_format_info(struct drm_device *dev,
+ const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+ const struct drm_format_info *info = NULL;
+
+ if (dev->mode_config.funcs->get_format_info)
+ info = dev->mode_config.funcs->get_format_info(dev, mode_cmd);
+
+ if (!info)
+ info = drm_format_info(mode_cmd->pixel_format);
+
+ return info;
+}
+EXPORT_SYMBOL(drm_get_format_info);
+
+/**
* drm_format_num_planes - get the number of planes for format
* @format: pixel format (DRM_FORMAT_*)
*
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index 94ddab41f24f..292930a5dcc2 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -126,11 +126,13 @@ int drm_mode_addfb(struct drm_device *dev,
return 0;
}
-static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
+static int framebuffer_check(struct drm_device *dev,
+ const struct drm_mode_fb_cmd2 *r)
{
const struct drm_format_info *info;
int i;
+ /* check if the format is supported at all */
info = __drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
if (!info) {
struct drm_format_name_buf format_name;
@@ -140,6 +142,9 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
return -EINVAL;
}
+ /* now let the driver pick its own format info */
+ info = drm_get_format_info(dev, r);
+
if (r->width == 0 || r->width % info->hsub) {
DRM_DEBUG_KMS("bad framebuffer width %u\n", r->width);
return -EINVAL;
@@ -263,7 +268,7 @@ drm_internal_framebuffer_create(struct drm_device *dev,
return ERR_PTR(-EINVAL);
}
- ret = framebuffer_check(r);
+ ret = framebuffer_check(dev, r);
if (ret)
return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
index 5b051859b8d3..f78df06a940d 100644
--- a/drivers/gpu/drm/drm_modeset_helper.c
+++ b/drivers/gpu/drm/drm_modeset_helper.c
@@ -75,7 +75,7 @@ void drm_helper_mode_fill_fb_struct(struct drm_device *dev,
int i;
fb->dev = dev;
- fb->format = drm_format_info(mode_cmd->pixel_format);
+ fb->format = drm_get_format_info(dev, mode_cmd);
fb->width = mode_cmd->width;
fb->height = mode_cmd->height;
for (i = 0; i < 4; i++) {
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index fcc08da850c8..6942e84b6edd 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -25,6 +25,9 @@
#include <linux/types.h>
#include <uapi/drm/drm_fourcc.h>
+struct drm_device;
+struct drm_mode_fb_cmd2;
+
/**
* struct drm_format_info - information about a DRM format
* @format: 4CC format identifier (DRM_FORMAT_*)
@@ -55,6 +58,9 @@ struct drm_format_name_buf {
const struct drm_format_info *__drm_format_info(u32 format);
const struct drm_format_info *drm_format_info(u32 format);
+const struct drm_format_info *
+drm_get_format_info(struct drm_device *dev,
+ const struct drm_mode_fb_cmd2 *mode_cmd);
uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
int drm_format_num_planes(uint32_t format);
int drm_format_plane_cpp(uint32_t format, int plane);
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index bf9991b20611..80bb7e49bf8e 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -34,6 +34,7 @@ struct drm_file;
struct drm_device;
struct drm_atomic_state;
struct drm_mode_fb_cmd2;
+struct drm_format_info;
/**
* struct drm_mode_config_funcs - basic driver provided mode setting functions
@@ -70,6 +71,20 @@ struct drm_mode_config_funcs {
const struct drm_mode_fb_cmd2 *mode_cmd);
/**
+ * @get_format_info:
+ *
+ * Allows a driver to return custom format information for special
+ * fb layouts (eg. ones with auxiliary compresssion control planes).
+ *
+ * RETURNS:
+ *
+ * The format information specific to the given fb metadata, or
+ * NULL if none is found.
+ */
+ const struct drm_format_info *(*get_format_info)(struct drm_device *dev,
+ const struct drm_mode_fb_cmd2 *mode_cmd);
+
+ /**
* @output_poll_changed:
*
* Callback used by helpers to inform the driver of output configuration
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 36/37] drm: Add mode_config .get_format_info() hook
2016-11-18 19:53 ` [PATCH 36/37] drm: Add mode_config .get_format_info() hook ville.syrjala
@ 2016-11-20 8:13 ` Laurent Pinchart
2016-11-21 13:18 ` Ville Syrjälä
2016-11-22 13:41 ` [PATCH v2 " ville.syrjala
1 sibling, 1 reply; 34+ messages in thread
From: Laurent Pinchart @ 2016-11-20 8:13 UTC (permalink / raw)
To: ville.syrjala; +Cc: Ben Widawsky, intel-gfx, dri-devel
Hi Ville,
Thank you for the patch.
On Friday 18 Nov 2016 21:53:12 ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Allow drivers to return a custom drm_format_info structure for special
> fb layouts. We'll use this for the compression control surface in i915.
>
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/drm_fb_cma_helper.c | 2 +-
> drivers/gpu/drm/drm_fourcc.c | 25 +++++++++++++++++++++++++
> drivers/gpu/drm/drm_framebuffer.c | 9 +++++++--
> drivers/gpu/drm/drm_modeset_helper.c | 2 +-
> include/drm/drm_fourcc.h | 6 ++++++
> include/drm/drm_mode_config.h | 15 +++++++++++++++
> 6 files changed, 55 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> b/drivers/gpu/drm/drm_fb_cma_helper.c index aab4465307ed..d7f8876cf5e9
> 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -186,7 +186,7 @@ struct drm_framebuffer
> *drm_fb_cma_create_with_funcs(struct drm_device *dev, int ret;
> int i;
>
> - info = drm_format_info(mode_cmd->pixel_format);
> + info = drm_get_format_info(dev, mode_cmd);
> if (!info)
> return ERR_PTR(-EINVAL);
>
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 90d2cc8da8eb..7cfaee689f0c 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -199,6 +199,31 @@ const struct drm_format_info *drm_format_info(u32
> format) EXPORT_SYMBOL(drm_format_info);
>
> /**
> + * drm_format_info - query information for a given framebuffer
> configuration
I assume you meant drm_get_format_info()
> + * @dev: DRM device
Do we need the dev pointer ?
> + * @mode_cmd: metadata from the userspace fb creation request
> + *
> + * Returns:
> + * The instance of struct drm_format_info that describes the pixel format,
> or
> + * NULL if the format is unsupported.
It would be useful to document how this function differs from
drm_format_info(). I also wonder whether it would make sense to completely
replace drm_format_info() to avoid keeping two separate but very similar
functions.
> + */
> +const struct drm_format_info *
> +drm_get_format_info(struct drm_device *dev,
> + const struct drm_mode_fb_cmd2 *mode_cmd)
> +{
> + const struct drm_format_info *info = NULL;
> +
> + if (dev->mode_config.funcs->get_format_info)
> + info = dev->mode_config.funcs->get_format_info(dev, mode_cmd);
> +
> + if (!info)
> + info = drm_format_info(mode_cmd->pixel_format);
> +
> + return info;
> +}
> +EXPORT_SYMBOL(drm_get_format_info);
> +
> +/**
> * drm_format_num_planes - get the number of planes for format
> * @format: pixel format (DRM_FORMAT_*)
> *
> diff --git a/drivers/gpu/drm/drm_framebuffer.c
> b/drivers/gpu/drm/drm_framebuffer.c index 94ddab41f24f..292930a5dcc2 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -126,11 +126,13 @@ int drm_mode_addfb(struct drm_device *dev,
> return 0;
> }
>
> -static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
> +static int framebuffer_check(struct drm_device *dev,
> + const struct drm_mode_fb_cmd2 *r)
> {
> const struct drm_format_info *info;
> int i;
>
> + /* check if the format is supported at all */
> info = __drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
> if (!info) {
> struct drm_format_name_buf format_name;
> @@ -140,6 +142,9 @@ static int framebuffer_check(const struct
> drm_mode_fb_cmd2 *r) return -EINVAL;
> }
>
> + /* now let the driver pick its own format info */
> + info = drm_get_format_info(dev, r);
> +
> if (r->width == 0 || r->width % info->hsub) {
> DRM_DEBUG_KMS("bad framebuffer width %u\n", r->width);
> return -EINVAL;
> @@ -263,7 +268,7 @@ drm_internal_framebuffer_create(struct drm_device *dev,
> return ERR_PTR(-EINVAL);
> }
>
> - ret = framebuffer_check(r);
> + ret = framebuffer_check(dev, r);
> if (ret)
> return ERR_PTR(ret);
>
> diff --git a/drivers/gpu/drm/drm_modeset_helper.c
> b/drivers/gpu/drm/drm_modeset_helper.c index 5b051859b8d3..f78df06a940d
> 100644
> --- a/drivers/gpu/drm/drm_modeset_helper.c
> +++ b/drivers/gpu/drm/drm_modeset_helper.c
> @@ -75,7 +75,7 @@ void drm_helper_mode_fill_fb_struct(struct drm_device
> *dev, int i;
>
> fb->dev = dev;
> - fb->format = drm_format_info(mode_cmd->pixel_format);
> + fb->format = drm_get_format_info(dev, mode_cmd);
> fb->width = mode_cmd->width;
> fb->height = mode_cmd->height;
> for (i = 0; i < 4; i++) {
> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index fcc08da850c8..6942e84b6edd 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -25,6 +25,9 @@
> #include <linux/types.h>
> #include <uapi/drm/drm_fourcc.h>
>
> +struct drm_device;
> +struct drm_mode_fb_cmd2;
> +
> /**
> * struct drm_format_info - information about a DRM format
> * @format: 4CC format identifier (DRM_FORMAT_*)
> @@ -55,6 +58,9 @@ struct drm_format_name_buf {
>
> const struct drm_format_info *__drm_format_info(u32 format);
> const struct drm_format_info *drm_format_info(u32 format);
> +const struct drm_format_info *
> +drm_get_format_info(struct drm_device *dev,
> + const struct drm_mode_fb_cmd2 *mode_cmd);
> uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
> int drm_format_num_planes(uint32_t format);
> int drm_format_plane_cpp(uint32_t format, int plane);
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index bf9991b20611..80bb7e49bf8e 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -34,6 +34,7 @@ struct drm_file;
> struct drm_device;
> struct drm_atomic_state;
> struct drm_mode_fb_cmd2;
> +struct drm_format_info;
>
> /**
> * struct drm_mode_config_funcs - basic driver provided mode setting
> functions @@ -70,6 +71,20 @@ struct drm_mode_config_funcs {
> const struct drm_mode_fb_cmd2
*mode_cmd);
>
> /**
> + * @get_format_info:
> + *
> + * Allows a driver to return custom format information for special
> + * fb layouts (eg. ones with auxiliary compresssion control planes).
> + *
> + * RETURNS:
> + *
> + * The format information specific to the given fb metadata, or
> + * NULL if none is found.
> + */
> + const struct drm_format_info *(*get_format_info)(struct drm_device
*dev,
> + const struct
drm_mode_fb_cmd2 *mode_cmd);
> +
> + /**
> * @output_poll_changed:
> *
> * Callback used by helpers to inform the driver of output
configuration
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 36/37] drm: Add mode_config .get_format_info() hook
2016-11-20 8:13 ` Laurent Pinchart
@ 2016-11-21 13:18 ` Ville Syrjälä
2016-11-21 13:23 ` Laurent Pinchart
0 siblings, 1 reply; 34+ messages in thread
From: Ville Syrjälä @ 2016-11-21 13:18 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: Ben Widawsky, intel-gfx, dri-devel
On Sun, Nov 20, 2016 at 10:13:10AM +0200, Laurent Pinchart wrote:
> Hi Ville,
>
> Thank you for the patch.
>
> On Friday 18 Nov 2016 21:53:12 ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Allow drivers to return a custom drm_format_info structure for special
> > fb layouts. We'll use this for the compression control surface in i915.
> >
> > Cc: Ben Widawsky <ben@bwidawsk.net>
> > Cc: intel-gfx@lists.freedesktop.org
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/drm_fb_cma_helper.c | 2 +-
> > drivers/gpu/drm/drm_fourcc.c | 25 +++++++++++++++++++++++++
> > drivers/gpu/drm/drm_framebuffer.c | 9 +++++++--
> > drivers/gpu/drm/drm_modeset_helper.c | 2 +-
> > include/drm/drm_fourcc.h | 6 ++++++
> > include/drm/drm_mode_config.h | 15 +++++++++++++++
> > 6 files changed, 55 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> > b/drivers/gpu/drm/drm_fb_cma_helper.c index aab4465307ed..d7f8876cf5e9
> > 100644
> > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> > @@ -186,7 +186,7 @@ struct drm_framebuffer
> > *drm_fb_cma_create_with_funcs(struct drm_device *dev, int ret;
> > int i;
> >
> > - info = drm_format_info(mode_cmd->pixel_format);
> > + info = drm_get_format_info(dev, mode_cmd);
> > if (!info)
> > return ERR_PTR(-EINVAL);
> >
> > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> > index 90d2cc8da8eb..7cfaee689f0c 100644
> > --- a/drivers/gpu/drm/drm_fourcc.c
> > +++ b/drivers/gpu/drm/drm_fourcc.c
> > @@ -199,6 +199,31 @@ const struct drm_format_info *drm_format_info(u32
> > format) EXPORT_SYMBOL(drm_format_info);
> >
> > /**
> > + * drm_format_info - query information for a given framebuffer
> > configuration
>
> I assume you meant drm_get_format_info()
Yes.
>
> > + * @dev: DRM device
>
> Do we need the dev pointer ?
Not at the moment. I was thinking we might allow drivers to return a
different set of formats based on the device type, but I'm not sure
that's all that useful since drivers will have to check for unsupported
formats anyway in .fb_create(). The only use case might be if you need
to select between two different format info structs based on the device
type, because you simply can't tell the formats apart based on the
mode_cmd. But that sort of thing feels like a bad idea to me, and might
as well just require that you must be able to tell formats that require
different format intos apart based on the mode_cmd (eg. by having
different modifiers on them).
So I guess we could just drop the 'dev' argument to make it harder for
people to make that sort of mistake.
>
> > + * @mode_cmd: metadata from the userspace fb creation request
> > + *
> > + * Returns:
> > + * The instance of struct drm_format_info that describes the pixel format,
> > or
> > + * NULL if the format is unsupported.
>
> It would be useful to document how this function differs from
> drm_format_info(). I also wonder whether it would make sense to completely
> replace drm_format_info() to avoid keeping two separate but very similar
> functions.
Yeah, that is basically what I was thinking. But I didn't feel like
doing that myself as it looked like that might involve actual work
in some of the drivers. I figured I'd leave it up to whoever cares
about said drivers.
>
> > + */
> > +const struct drm_format_info *
> > +drm_get_format_info(struct drm_device *dev,
> > + const struct drm_mode_fb_cmd2 *mode_cmd)
> > +{
> > + const struct drm_format_info *info = NULL;
> > +
> > + if (dev->mode_config.funcs->get_format_info)
> > + info = dev->mode_config.funcs->get_format_info(dev, mode_cmd);
> > +
> > + if (!info)
> > + info = drm_format_info(mode_cmd->pixel_format);
> > +
> > + return info;
> > +}
> > +EXPORT_SYMBOL(drm_get_format_info);
> > +
> > +/**
> > * drm_format_num_planes - get the number of planes for format
> > * @format: pixel format (DRM_FORMAT_*)
> > *
> > diff --git a/drivers/gpu/drm/drm_framebuffer.c
> > b/drivers/gpu/drm/drm_framebuffer.c index 94ddab41f24f..292930a5dcc2 100644
> > --- a/drivers/gpu/drm/drm_framebuffer.c
> > +++ b/drivers/gpu/drm/drm_framebuffer.c
> > @@ -126,11 +126,13 @@ int drm_mode_addfb(struct drm_device *dev,
> > return 0;
> > }
> >
> > -static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
> > +static int framebuffer_check(struct drm_device *dev,
> > + const struct drm_mode_fb_cmd2 *r)
> > {
> > const struct drm_format_info *info;
> > int i;
> >
> > + /* check if the format is supported at all */
> > info = __drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
> > if (!info) {
> > struct drm_format_name_buf format_name;
> > @@ -140,6 +142,9 @@ static int framebuffer_check(const struct
> > drm_mode_fb_cmd2 *r) return -EINVAL;
> > }
> >
> > + /* now let the driver pick its own format info */
> > + info = drm_get_format_info(dev, r);
> > +
> > if (r->width == 0 || r->width % info->hsub) {
> > DRM_DEBUG_KMS("bad framebuffer width %u\n", r->width);
> > return -EINVAL;
> > @@ -263,7 +268,7 @@ drm_internal_framebuffer_create(struct drm_device *dev,
> > return ERR_PTR(-EINVAL);
> > }
> >
> > - ret = framebuffer_check(r);
> > + ret = framebuffer_check(dev, r);
> > if (ret)
> > return ERR_PTR(ret);
> >
> > diff --git a/drivers/gpu/drm/drm_modeset_helper.c
> > b/drivers/gpu/drm/drm_modeset_helper.c index 5b051859b8d3..f78df06a940d
> > 100644
> > --- a/drivers/gpu/drm/drm_modeset_helper.c
> > +++ b/drivers/gpu/drm/drm_modeset_helper.c
> > @@ -75,7 +75,7 @@ void drm_helper_mode_fill_fb_struct(struct drm_device
> > *dev, int i;
> >
> > fb->dev = dev;
> > - fb->format = drm_format_info(mode_cmd->pixel_format);
> > + fb->format = drm_get_format_info(dev, mode_cmd);
> > fb->width = mode_cmd->width;
> > fb->height = mode_cmd->height;
> > for (i = 0; i < 4; i++) {
> > diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> > index fcc08da850c8..6942e84b6edd 100644
> > --- a/include/drm/drm_fourcc.h
> > +++ b/include/drm/drm_fourcc.h
> > @@ -25,6 +25,9 @@
> > #include <linux/types.h>
> > #include <uapi/drm/drm_fourcc.h>
> >
> > +struct drm_device;
> > +struct drm_mode_fb_cmd2;
> > +
> > /**
> > * struct drm_format_info - information about a DRM format
> > * @format: 4CC format identifier (DRM_FORMAT_*)
> > @@ -55,6 +58,9 @@ struct drm_format_name_buf {
> >
> > const struct drm_format_info *__drm_format_info(u32 format);
> > const struct drm_format_info *drm_format_info(u32 format);
> > +const struct drm_format_info *
> > +drm_get_format_info(struct drm_device *dev,
> > + const struct drm_mode_fb_cmd2 *mode_cmd);
> > uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
> > int drm_format_num_planes(uint32_t format);
> > int drm_format_plane_cpp(uint32_t format, int plane);
> > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> > index bf9991b20611..80bb7e49bf8e 100644
> > --- a/include/drm/drm_mode_config.h
> > +++ b/include/drm/drm_mode_config.h
> > @@ -34,6 +34,7 @@ struct drm_file;
> > struct drm_device;
> > struct drm_atomic_state;
> > struct drm_mode_fb_cmd2;
> > +struct drm_format_info;
> >
> > /**
> > * struct drm_mode_config_funcs - basic driver provided mode setting
> > functions @@ -70,6 +71,20 @@ struct drm_mode_config_funcs {
> > const struct drm_mode_fb_cmd2
> *mode_cmd);
> >
> > /**
> > + * @get_format_info:
> > + *
> > + * Allows a driver to return custom format information for special
> > + * fb layouts (eg. ones with auxiliary compresssion control planes).
> > + *
> > + * RETURNS:
> > + *
> > + * The format information specific to the given fb metadata, or
> > + * NULL if none is found.
> > + */
> > + const struct drm_format_info *(*get_format_info)(struct drm_device
> *dev,
> > + const struct
> drm_mode_fb_cmd2 *mode_cmd);
> > +
> > + /**
> > * @output_poll_changed:
> > *
> > * Callback used by helpers to inform the driver of output
> configuration
>
> --
> Regards,
>
> Laurent Pinchart
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 36/37] drm: Add mode_config .get_format_info() hook
2016-11-21 13:18 ` Ville Syrjälä
@ 2016-11-21 13:23 ` Laurent Pinchart
2016-11-21 13:31 ` Ville Syrjälä
0 siblings, 1 reply; 34+ messages in thread
From: Laurent Pinchart @ 2016-11-21 13:23 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Ben Widawsky, intel-gfx, dri-devel
Hi Ville,
On Monday 21 Nov 2016 15:18:23 Ville Syrjälä wrote:
> On Sun, Nov 20, 2016 at 10:13:10AM +0200, Laurent Pinchart wrote:
> > On Friday 18 Nov 2016 21:53:12 ville.syrjala@linux.intel.com wrote:
> >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >> Allow drivers to return a custom drm_format_info structure for special
> >> fb layouts. We'll use this for the compression control surface in i915.
> >>
> >> Cc: Ben Widawsky <ben@bwidawsk.net>
> >> Cc: intel-gfx@lists.freedesktop.org
> >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> ---
> >>
> >> drivers/gpu/drm/drm_fb_cma_helper.c | 2 +-
> >> drivers/gpu/drm/drm_fourcc.c | 25 +++++++++++++++++++++++++
> >> drivers/gpu/drm/drm_framebuffer.c | 9 +++++++--
> >> drivers/gpu/drm/drm_modeset_helper.c | 2 +-
> >> include/drm/drm_fourcc.h | 6 ++++++
> >> include/drm/drm_mode_config.h | 15 +++++++++++++++
> >> 6 files changed, 55 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> >> b/drivers/gpu/drm/drm_fb_cma_helper.c index aab4465307ed..d7f8876cf5e9
> >> 100644
> >> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> >> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> >> @@ -186,7 +186,7 @@ struct drm_framebuffer
> >> *drm_fb_cma_create_with_funcs(struct drm_device *dev, int ret;
> >>
> >> int i;
> >>
> >> - info = drm_format_info(mode_cmd->pixel_format);
> >> + info = drm_get_format_info(dev, mode_cmd);
> >> if (!info)
> >> return ERR_PTR(-EINVAL);
> >>
> >> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> >> index 90d2cc8da8eb..7cfaee689f0c 100644
> >> --- a/drivers/gpu/drm/drm_fourcc.c
> >> +++ b/drivers/gpu/drm/drm_fourcc.c
> >> @@ -199,6 +199,31 @@ const struct drm_format_info *drm_format_info(u32
> >> format)
> >> EXPORT_SYMBOL(drm_format_info);
> >>
> >> /**
> >> + * drm_format_info - query information for a given framebuffer
> >> configuration
> >
> > I assume you meant drm_get_format_info()
>
> Yes.
>
> >> + * @dev: DRM device
> >
> > Do we need the dev pointer ?
>
> Not at the moment. I was thinking we might allow drivers to return a
> different set of formats based on the device type, but I'm not sure
> that's all that useful since drivers will have to check for unsupported
> formats anyway in .fb_create(). The only use case might be if you need
> to select between two different format info structs based on the device
> type, because you simply can't tell the formats apart based on the
> mode_cmd. But that sort of thing feels like a bad idea to me, and might
> as well just require that you must be able to tell formats that require
> different format intos apart based on the mode_cmd (eg. by having
> different modifiers on them).
>
> So I guess we could just drop the 'dev' argument to make it harder for
> people to make that sort of mistake.
I think that's a good idea, yes.
> >> + * @mode_cmd: metadata from the userspace fb creation request
> >> + *
> >> + * Returns:
> >> + * The instance of struct drm_format_info that describes the pixel
> >> format, or
> >> + * NULL if the format is unsupported.
> >
> > It would be useful to document how this function differs from
> > drm_format_info(). I also wonder whether it would make sense to completely
> > replace drm_format_info() to avoid keeping two separate but very similar
> > functions.
>
> Yeah, that is basically what I was thinking. But I didn't feel like
> doing that myself as it looked like that might involve actual work
> in some of the drivers. I figured I'd leave it up to whoever cares
> about said drivers.
Which driver(s) are you thinking about ? If we want to make
drm_get_format_info() the default we obviously need to pass modifiers
directly, as in most cases we won't have a struct drm_mode_fb_cmd2 to pass to
the function. If we remove the dev argument you could just pass NULL modifiers
in most cases, I don't think that would involve much rework in drivers.
> >> + */
> >> +const struct drm_format_info *
> >> +drm_get_format_info(struct drm_device *dev,
> >> + const struct drm_mode_fb_cmd2 *mode_cmd)
> >> +{
> >> + const struct drm_format_info *info = NULL;
> >> +
> >> + if (dev->mode_config.funcs->get_format_info)
> >> + info = dev->mode_config.funcs->get_format_info(dev, mode_cmd);
> >> +
> >> + if (!info)
> >> + info = drm_format_info(mode_cmd->pixel_format);
> >> +
> >> + return info;
> >> +}
> >> +EXPORT_SYMBOL(drm_get_format_info);
--
Regards,
Laurent Pinchart
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 36/37] drm: Add mode_config .get_format_info() hook
2016-11-21 13:23 ` Laurent Pinchart
@ 2016-11-21 13:31 ` Ville Syrjälä
2016-11-21 13:42 ` Laurent Pinchart
0 siblings, 1 reply; 34+ messages in thread
From: Ville Syrjälä @ 2016-11-21 13:31 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: Ben Widawsky, intel-gfx, dri-devel
On Mon, Nov 21, 2016 at 03:23:19PM +0200, Laurent Pinchart wrote:
> Hi Ville,
>
> On Monday 21 Nov 2016 15:18:23 Ville Syrjälä wrote:
> > On Sun, Nov 20, 2016 at 10:13:10AM +0200, Laurent Pinchart wrote:
> > > On Friday 18 Nov 2016 21:53:12 ville.syrjala@linux.intel.com wrote:
> > >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >>
> > >> Allow drivers to return a custom drm_format_info structure for special
> > >> fb layouts. We'll use this for the compression control surface in i915.
> > >>
> > >> Cc: Ben Widawsky <ben@bwidawsk.net>
> > >> Cc: intel-gfx@lists.freedesktop.org
> > >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >> ---
> > >>
> > >> drivers/gpu/drm/drm_fb_cma_helper.c | 2 +-
> > >> drivers/gpu/drm/drm_fourcc.c | 25 +++++++++++++++++++++++++
> > >> drivers/gpu/drm/drm_framebuffer.c | 9 +++++++--
> > >> drivers/gpu/drm/drm_modeset_helper.c | 2 +-
> > >> include/drm/drm_fourcc.h | 6 ++++++
> > >> include/drm/drm_mode_config.h | 15 +++++++++++++++
> > >> 6 files changed, 55 insertions(+), 4 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> > >> b/drivers/gpu/drm/drm_fb_cma_helper.c index aab4465307ed..d7f8876cf5e9
> > >> 100644
> > >> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > >> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> > >> @@ -186,7 +186,7 @@ struct drm_framebuffer
> > >> *drm_fb_cma_create_with_funcs(struct drm_device *dev, int ret;
> > >>
> > >> int i;
> > >>
> > >> - info = drm_format_info(mode_cmd->pixel_format);
> > >> + info = drm_get_format_info(dev, mode_cmd);
> > >> if (!info)
> > >> return ERR_PTR(-EINVAL);
> > >>
> > >> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> > >> index 90d2cc8da8eb..7cfaee689f0c 100644
> > >> --- a/drivers/gpu/drm/drm_fourcc.c
> > >> +++ b/drivers/gpu/drm/drm_fourcc.c
> > >> @@ -199,6 +199,31 @@ const struct drm_format_info *drm_format_info(u32
> > >> format)
> > >> EXPORT_SYMBOL(drm_format_info);
> > >>
> > >> /**
> > >> + * drm_format_info - query information for a given framebuffer
> > >> configuration
> > >
> > > I assume you meant drm_get_format_info()
> >
> > Yes.
> >
> > >> + * @dev: DRM device
> > >
> > > Do we need the dev pointer ?
> >
> > Not at the moment. I was thinking we might allow drivers to return a
> > different set of formats based on the device type, but I'm not sure
> > that's all that useful since drivers will have to check for unsupported
> > formats anyway in .fb_create(). The only use case might be if you need
> > to select between two different format info structs based on the device
> > type, because you simply can't tell the formats apart based on the
> > mode_cmd. But that sort of thing feels like a bad idea to me, and might
> > as well just require that you must be able to tell formats that require
> > different format intos apart based on the mode_cmd (eg. by having
> > different modifiers on them).
> >
> > So I guess we could just drop the 'dev' argument to make it harder for
> > people to make that sort of mistake.
>
> I think that's a good idea, yes.
>
> > >> + * @mode_cmd: metadata from the userspace fb creation request
> > >> + *
> > >> + * Returns:
> > >> + * The instance of struct drm_format_info that describes the pixel
> > >> format, or
> > >> + * NULL if the format is unsupported.
> > >
> > > It would be useful to document how this function differs from
> > > drm_format_info(). I also wonder whether it would make sense to completely
> > > replace drm_format_info() to avoid keeping two separate but very similar
> > > functions.
> >
> > Yeah, that is basically what I was thinking. But I didn't feel like
> > doing that myself as it looked like that might involve actual work
> > in some of the drivers. I figured I'd leave it up to whoever cares
> > about said drivers.
>
> Which driver(s) are you thinking about ?
The ones that my cocci stuff couldn't convert over to fb->format.
> If we want to make
> drm_get_format_info() the default we obviously need to pass modifiers
> directly, as in most cases we won't have a struct drm_mode_fb_cmd2 to pass to
> the function. If we remove the dev argument you could just pass NULL modifiers
> in most cases, I don't think that would involve much rework in drivers.
fb->format is probably the right choice in most cases. But some drivers
seemed to have some kind of internal format info struct instead which
was in the way of doing a trivial conversion. I didn't want to start
doing non-trivial conversions since the series was already way too big
as is.
>
> > >> + */
> > >> +const struct drm_format_info *
> > >> +drm_get_format_info(struct drm_device *dev,
> > >> + const struct drm_mode_fb_cmd2 *mode_cmd)
> > >> +{
> > >> + const struct drm_format_info *info = NULL;
> > >> +
> > >> + if (dev->mode_config.funcs->get_format_info)
> > >> + info = dev->mode_config.funcs->get_format_info(dev, mode_cmd);
> > >> +
> > >> + if (!info)
> > >> + info = drm_format_info(mode_cmd->pixel_format);
> > >> +
> > >> + return info;
> > >> +}
> > >> +EXPORT_SYMBOL(drm_get_format_info);
>
> --
> Regards,
>
> Laurent Pinchart
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 36/37] drm: Add mode_config .get_format_info() hook
2016-11-21 13:31 ` Ville Syrjälä
@ 2016-11-21 13:42 ` Laurent Pinchart
2016-11-21 14:25 ` Ville Syrjälä
0 siblings, 1 reply; 34+ messages in thread
From: Laurent Pinchart @ 2016-11-21 13:42 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Ben Widawsky, intel-gfx, dri-devel
Hi Ville,
On Monday 21 Nov 2016 15:31:57 Ville Syrjälä wrote:
> On Mon, Nov 21, 2016 at 03:23:19PM +0200, Laurent Pinchart wrote:
> > On Monday 21 Nov 2016 15:18:23 Ville Syrjälä wrote:
> >> On Sun, Nov 20, 2016 at 10:13:10AM +0200, Laurent Pinchart wrote:
> >>> On Friday 18 Nov 2016 21:53:12 ville.syrjala@linux.intel.com wrote:
> >>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>
> >>>> Allow drivers to return a custom drm_format_info structure for
> >>>> special fb layouts. We'll use this for the compression control surface
> >>>> in i915.
> >>>>
> >>>> Cc: Ben Widawsky <ben@bwidawsk.net>
> >>>> Cc: intel-gfx@lists.freedesktop.org
> >>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>> ---
> >>>>
> >>>> drivers/gpu/drm/drm_fb_cma_helper.c | 2 +-
> >>>> drivers/gpu/drm/drm_fourcc.c | 25 +++++++++++++++++++++++++
> >>>> drivers/gpu/drm/drm_framebuffer.c | 9 +++++++--
> >>>> drivers/gpu/drm/drm_modeset_helper.c | 2 +-
> >>>> include/drm/drm_fourcc.h | 6 ++++++
> >>>> include/drm/drm_mode_config.h | 15 +++++++++++++++
> >>>> 6 files changed, 55 insertions(+), 4 deletions(-)
[snip]
> >>>> diff --git a/drivers/gpu/drm/drm_fourcc.c
> >>>> b/drivers/gpu/drm/drm_fourcc.c
> >>>> index 90d2cc8da8eb..7cfaee689f0c 100644
> >>>> --- a/drivers/gpu/drm/drm_fourcc.c
> >>>> +++ b/drivers/gpu/drm/drm_fourcc.c
> >>>> @@ -199,6 +199,31 @@ const struct drm_format_info
> >>>> *drm_format_info(u32 format)
> >>>> EXPORT_SYMBOL(drm_format_info);
> >>>>
> >>>> /**
> >>>> + * drm_format_info - query information for a given framebuffer
> >>>> configuration
> >>>
> >>> I assume you meant drm_get_format_info()
> >>
> >> Yes.
> >>
> >>>> + * @dev: DRM device
> >>>
> >>> Do we need the dev pointer ?
> >>
> >> Not at the moment. I was thinking we might allow drivers to return a
> >> different set of formats based on the device type, but I'm not sure
> >> that's all that useful since drivers will have to check for unsupported
> >> formats anyway in .fb_create(). The only use case might be if you need
> >> to select between two different format info structs based on the device
> >> type, because you simply can't tell the formats apart based on the
> >> mode_cmd. But that sort of thing feels like a bad idea to me, and might
> >> as well just require that you must be able to tell formats that require
> >> different format intos apart based on the mode_cmd (eg. by having
> >> different modifiers on them).
> >>
> >> So I guess we could just drop the 'dev' argument to make it harder for
> >> people to make that sort of mistake.
> >
> > I think that's a good idea, yes.
> >
> >>>> + * @mode_cmd: metadata from the userspace fb creation request
> >>>> + *
> >>>> + * Returns:
> >>>> + * The instance of struct drm_format_info that describes the pixel
> >>>> format, or
> >>>> + * NULL if the format is unsupported.
> >>>
> >>> It would be useful to document how this function differs from
> >>> drm_format_info(). I also wonder whether it would make sense to
> >>> completely replace drm_format_info() to avoid keeping two separate but
> >>> very similar functions.
> >>
> >> Yeah, that is basically what I was thinking. But I didn't feel like
> >> doing that myself as it looked like that might involve actual work
> >> in some of the drivers. I figured I'd leave it up to whoever cares
> >> about said drivers.
> >
> > Which driver(s) are you thinking about ?
>
> The ones that my cocci stuff couldn't convert over to fb->format.
How about at least making drm_get_format_info() the default but converting
what can be converted with coccinelle, and marking drm_format_info() as
deprecated ?
> > If we want to make drm_get_format_info() the default we obviously need to
> > pass modifiers directly, as in most cases we won't have a struct
> > drm_mode_fb_cmd2 to pass to the function. If we remove the dev argument
> > you could just pass NULL modifiers in most cases, I don't think that would
> > involve much rework in drivers.
>
> fb->format is probably the right choice in most cases. But some drivers
> seemed to have some kind of internal format info struct instead which
> was in the way of doing a trivial conversion. I didn't want to start
> doing non-trivial conversions since the series was already way too big
> as is.
That's an interesting point I wanted to also mention. We have drivers that
include formats information tables duplicating the one in the DRM core, with
additional driver-specific information (see rcar_du_format_info() in
drivers/gpu/drm/rcar-du/rcar_du_kms.c for instance). I wonder whether it would
be possible to come up with a simple API that would allow providing those
driver-specific data to the core, and get them back from the
drm_get_format_info() function.
--
Regards,
Laurent Pinchart
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 36/37] drm: Add mode_config .get_format_info() hook
2016-11-21 13:42 ` Laurent Pinchart
@ 2016-11-21 14:25 ` Ville Syrjälä
0 siblings, 0 replies; 34+ messages in thread
From: Ville Syrjälä @ 2016-11-21 14:25 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: Ben Widawsky, intel-gfx, dri-devel
On Mon, Nov 21, 2016 at 03:42:34PM +0200, Laurent Pinchart wrote:
> Hi Ville,
>
> On Monday 21 Nov 2016 15:31:57 Ville Syrjälä wrote:
> > On Mon, Nov 21, 2016 at 03:23:19PM +0200, Laurent Pinchart wrote:
> > > On Monday 21 Nov 2016 15:18:23 Ville Syrjälä wrote:
> > >> On Sun, Nov 20, 2016 at 10:13:10AM +0200, Laurent Pinchart wrote:
> > >>> On Friday 18 Nov 2016 21:53:12 ville.syrjala@linux.intel.com wrote:
> > >>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >>>>
> > >>>> Allow drivers to return a custom drm_format_info structure for
> > >>>> special fb layouts. We'll use this for the compression control surface
> > >>>> in i915.
> > >>>>
> > >>>> Cc: Ben Widawsky <ben@bwidawsk.net>
> > >>>> Cc: intel-gfx@lists.freedesktop.org
> > >>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >>>> ---
> > >>>>
> > >>>> drivers/gpu/drm/drm_fb_cma_helper.c | 2 +-
> > >>>> drivers/gpu/drm/drm_fourcc.c | 25 +++++++++++++++++++++++++
> > >>>> drivers/gpu/drm/drm_framebuffer.c | 9 +++++++--
> > >>>> drivers/gpu/drm/drm_modeset_helper.c | 2 +-
> > >>>> include/drm/drm_fourcc.h | 6 ++++++
> > >>>> include/drm/drm_mode_config.h | 15 +++++++++++++++
> > >>>> 6 files changed, 55 insertions(+), 4 deletions(-)
>
> [snip]
>
> > >>>> diff --git a/drivers/gpu/drm/drm_fourcc.c
> > >>>> b/drivers/gpu/drm/drm_fourcc.c
> > >>>> index 90d2cc8da8eb..7cfaee689f0c 100644
> > >>>> --- a/drivers/gpu/drm/drm_fourcc.c
> > >>>> +++ b/drivers/gpu/drm/drm_fourcc.c
> > >>>> @@ -199,6 +199,31 @@ const struct drm_format_info
> > >>>> *drm_format_info(u32 format)
> > >>>> EXPORT_SYMBOL(drm_format_info);
> > >>>>
> > >>>> /**
> > >>>> + * drm_format_info - query information for a given framebuffer
> > >>>> configuration
> > >>>
> > >>> I assume you meant drm_get_format_info()
> > >>
> > >> Yes.
> > >>
> > >>>> + * @dev: DRM device
> > >>>
> > >>> Do we need the dev pointer ?
> > >>
> > >> Not at the moment. I was thinking we might allow drivers to return a
> > >> different set of formats based on the device type, but I'm not sure
> > >> that's all that useful since drivers will have to check for unsupported
> > >> formats anyway in .fb_create(). The only use case might be if you need
> > >> to select between two different format info structs based on the device
> > >> type, because you simply can't tell the formats apart based on the
> > >> mode_cmd. But that sort of thing feels like a bad idea to me, and might
> > >> as well just require that you must be able to tell formats that require
> > >> different format intos apart based on the mode_cmd (eg. by having
> > >> different modifiers on them).
> > >>
> > >> So I guess we could just drop the 'dev' argument to make it harder for
> > >> people to make that sort of mistake.
> > >
> > > I think that's a good idea, yes.
> > >
> > >>>> + * @mode_cmd: metadata from the userspace fb creation request
> > >>>> + *
> > >>>> + * Returns:
> > >>>> + * The instance of struct drm_format_info that describes the pixel
> > >>>> format, or
> > >>>> + * NULL if the format is unsupported.
> > >>>
> > >>> It would be useful to document how this function differs from
> > >>> drm_format_info(). I also wonder whether it would make sense to
> > >>> completely replace drm_format_info() to avoid keeping two separate but
> > >>> very similar functions.
> > >>
> > >> Yeah, that is basically what I was thinking. But I didn't feel like
> > >> doing that myself as it looked like that might involve actual work
> > >> in some of the drivers. I figured I'd leave it up to whoever cares
> > >> about said drivers.
> > >
> > > Which driver(s) are you thinking about ?
> >
> > The ones that my cocci stuff couldn't convert over to fb->format.
>
> How about at least making drm_get_format_info() the default but converting
> what can be converted with coccinelle, and marking drm_format_info() as
> deprecated ?
I think I already did everything except the "mark as deprecated" part.
And adding that last bit into the patch would be trivial.
>
> > > If we want to make drm_get_format_info() the default we obviously need to
> > > pass modifiers directly, as in most cases we won't have a struct
> > > drm_mode_fb_cmd2 to pass to the function. If we remove the dev argument
> > > you could just pass NULL modifiers in most cases, I don't think that would
> > > involve much rework in drivers.
> >
> > fb->format is probably the right choice in most cases. But some drivers
> > seemed to have some kind of internal format info struct instead which
> > was in the way of doing a trivial conversion. I didn't want to start
> > doing non-trivial conversions since the series was already way too big
> > as is.
>
> That's an interesting point I wanted to also mention. We have drivers that
> include formats information tables duplicating the one in the DRM core, with
> additional driver-specific information (see rcar_du_format_info() in
> drivers/gpu/drm/rcar-du/rcar_du_kms.c for instance). I wonder whether it would
> be possible to come up with a simple API that would allow providing those
> driver-specific data to the core, and get them back from the
> drm_get_format_info() function.
I suppose drivers could just embed the drm_format_info into some bigger
structure which has additional information. One thing that makes that a
little harder is the fact that the regular format info structures aren't
exported to drivers in a way that they could just:
static const struct my_format_info format_info_foo {
.base = drm_format_info_foo,
...
};
So they'd have to duplicate the information which is maybe a little
error prone. OTOH that problem already exists to some degree with my
.get_format_info() hook (which somewhat makes me think we should just
put all of that stuff into drm_fourcc.c).
But I suppose exporting the drm_format_info structs could be done as
well, and then drivers could do whatever they want.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 36/37] drm: Add mode_config .get_format_info() hook
2016-11-18 19:53 ` [PATCH 36/37] drm: Add mode_config .get_format_info() hook ville.syrjala
2016-11-20 8:13 ` Laurent Pinchart
@ 2016-11-22 13:41 ` ville.syrjala
1 sibling, 0 replies; 34+ messages in thread
From: ville.syrjala @ 2016-11-22 13:41 UTC (permalink / raw)
To: dri-devel; +Cc: Ben Widawsky, intel-gfx, Laurent Pinchart
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Allow drivers to return a custom drm_format_info structure for special
fb layouts. We'll use this for the compression control surface in i915.
v2: Fix drm_get_format_info() kernel doc (Laurent)
Don't pass 'dev' to the new hook (Laurent)
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_fb_cma_helper.c | 2 +-
drivers/gpu/drm/drm_fourcc.c | 25 +++++++++++++++++++++++++
drivers/gpu/drm/drm_framebuffer.c | 9 +++++++--
drivers/gpu/drm/drm_modeset_helper.c | 2 +-
include/drm/drm_fourcc.h | 6 ++++++
include/drm/drm_mode_config.h | 14 ++++++++++++++
6 files changed, 54 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index aab4465307ed..d7f8876cf5e9 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -186,7 +186,7 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,
int ret;
int i;
- info = drm_format_info(mode_cmd->pixel_format);
+ info = drm_get_format_info(dev, mode_cmd);
if (!info)
return ERR_PTR(-EINVAL);
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 90d2cc8da8eb..f9b6445e846a 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -199,6 +199,31 @@ const struct drm_format_info *drm_format_info(u32 format)
EXPORT_SYMBOL(drm_format_info);
/**
+ * drm_get_format_info - query information for a given framebuffer configuration
+ * @dev: DRM device
+ * @mode_cmd: metadata from the userspace fb creation request
+ *
+ * Returns:
+ * The instance of struct drm_format_info that describes the pixel format, or
+ * NULL if the format is unsupported.
+ */
+const struct drm_format_info *
+drm_get_format_info(struct drm_device *dev,
+ const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+ const struct drm_format_info *info = NULL;
+
+ if (dev->mode_config.funcs->get_format_info)
+ info = dev->mode_config.funcs->get_format_info(mode_cmd);
+
+ if (!info)
+ info = drm_format_info(mode_cmd->pixel_format);
+
+ return info;
+}
+EXPORT_SYMBOL(drm_get_format_info);
+
+/**
* drm_format_num_planes - get the number of planes for format
* @format: pixel format (DRM_FORMAT_*)
*
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index 94ddab41f24f..292930a5dcc2 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -126,11 +126,13 @@ int drm_mode_addfb(struct drm_device *dev,
return 0;
}
-static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
+static int framebuffer_check(struct drm_device *dev,
+ const struct drm_mode_fb_cmd2 *r)
{
const struct drm_format_info *info;
int i;
+ /* check if the format is supported at all */
info = __drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
if (!info) {
struct drm_format_name_buf format_name;
@@ -140,6 +142,9 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
return -EINVAL;
}
+ /* now let the driver pick its own format info */
+ info = drm_get_format_info(dev, r);
+
if (r->width == 0 || r->width % info->hsub) {
DRM_DEBUG_KMS("bad framebuffer width %u\n", r->width);
return -EINVAL;
@@ -263,7 +268,7 @@ drm_internal_framebuffer_create(struct drm_device *dev,
return ERR_PTR(-EINVAL);
}
- ret = framebuffer_check(r);
+ ret = framebuffer_check(dev, r);
if (ret)
return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
index 5b051859b8d3..f78df06a940d 100644
--- a/drivers/gpu/drm/drm_modeset_helper.c
+++ b/drivers/gpu/drm/drm_modeset_helper.c
@@ -75,7 +75,7 @@ void drm_helper_mode_fill_fb_struct(struct drm_device *dev,
int i;
fb->dev = dev;
- fb->format = drm_format_info(mode_cmd->pixel_format);
+ fb->format = drm_get_format_info(dev, mode_cmd);
fb->width = mode_cmd->width;
fb->height = mode_cmd->height;
for (i = 0; i < 4; i++) {
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index fcc08da850c8..6942e84b6edd 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -25,6 +25,9 @@
#include <linux/types.h>
#include <uapi/drm/drm_fourcc.h>
+struct drm_device;
+struct drm_mode_fb_cmd2;
+
/**
* struct drm_format_info - information about a DRM format
* @format: 4CC format identifier (DRM_FORMAT_*)
@@ -55,6 +58,9 @@ struct drm_format_name_buf {
const struct drm_format_info *__drm_format_info(u32 format);
const struct drm_format_info *drm_format_info(u32 format);
+const struct drm_format_info *
+drm_get_format_info(struct drm_device *dev,
+ const struct drm_mode_fb_cmd2 *mode_cmd);
uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
int drm_format_num_planes(uint32_t format);
int drm_format_plane_cpp(uint32_t format, int plane);
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index bf9991b20611..3af17a3a69fb 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -34,6 +34,7 @@ struct drm_file;
struct drm_device;
struct drm_atomic_state;
struct drm_mode_fb_cmd2;
+struct drm_format_info;
/**
* struct drm_mode_config_funcs - basic driver provided mode setting functions
@@ -70,6 +71,19 @@ struct drm_mode_config_funcs {
const struct drm_mode_fb_cmd2 *mode_cmd);
/**
+ * @get_format_info:
+ *
+ * Allows a driver to return custom format information for special
+ * fb layouts (eg. ones with auxiliary compresssion control planes).
+ *
+ * RETURNS:
+ *
+ * The format information specific to the given fb metadata, or
+ * NULL if none is found.
+ */
+ const struct drm_format_info *(*get_format_info)(const struct drm_mode_fb_cmd2 *mode_cmd);
+
+ /**
* @output_poll_changed:
*
* Callback used by helpers to inform the driver of output configuration
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 37/37] drm/i915: Implement .get_format_info() hook for CCS
2016-11-18 19:52 [PATCH v2 00/37] drm: Deduplicate fb format information (v2) ville.syrjala
` (6 preceding siblings ...)
2016-11-18 19:53 ` [PATCH 36/37] drm: Add mode_config .get_format_info() hook ville.syrjala
@ 2016-11-18 19:53 ` ville.syrjala
2016-11-18 23:31 ` Ben Widawsky
2016-11-21 8:42 ` [Intel-gfx] " Tvrtko Ursulin
2016-11-21 11:18 ` [PATCH v2 00/37] drm: Deduplicate fb format information (v2) Christian König
2016-12-14 21:37 ` Ville Syrjälä
9 siblings, 2 replies; 34+ messages in thread
From: ville.syrjala @ 2016-11-18 19:53 UTC (permalink / raw)
To: dri-devel; +Cc: Ben Widawsky, intel-gfx, Laurent Pinchart
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
By providing our own format information for the CCS formats, we should
be able to make framebuffer_check() do the right thing for the CCS
surface as well.
Note that we'll return the same format info for both Y and Yf tiled
format as that's what happens with the non-CCS Y vs. Yf as well. If
desired, we could potentially return a unique pointer for each
pixel_format+tiling+ccs combination, in which case we immediately be
able to tell if any of that stuff changed by just comparing the
pointers. But that does sound a bit wasteful space wise.
Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++++++++++++++++++++++++
include/uapi/drm/drm_fourcc.h | 3 +++
2 files changed, 40 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7b7135be3b9e..de6909770c68 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2488,6 +2488,42 @@ static unsigned int intel_fb_modifier_to_tiling(uint64_t fb_modifier)
}
}
+static const struct drm_format_info ccs_formats[] = {
+ { .format = DRM_FORMAT_XRGB8888, .depth = 24, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
+ { .format = DRM_FORMAT_XBGR8888, .depth = 24, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
+ { .format = DRM_FORMAT_ARGB8888, .depth = 32, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
+ { .format = DRM_FORMAT_ABGR8888, .depth = 32, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
+};
+
+static const struct drm_format_info *
+lookup_format_info(const struct drm_format_info formats[],
+ int num_formats, u32 format)
+{
+ int i;
+
+ for (i = 0; i < num_formats; i++) {
+ if (formats[i].format == format)
+ return &formats[i];
+ }
+
+ return NULL;
+}
+
+static const struct drm_format_info *
+intel_get_format_info(struct drm_device *dev,
+ const struct drm_mode_fb_cmd2 *cmd)
+{
+ switch (cmd->modifier[0]) {
+ case I915_FORMAT_MOD_Y_TILED_CCS:
+ case I915_FORMAT_MOD_Yf_TILED_CCS:
+ return lookup_format_info(ccs_formats,
+ ARRAY_SIZE(ccs_formats),
+ cmd->pixel_format);
+ default:
+ return NULL;
+ }
+}
+
static int
intel_fill_fb_info(struct drm_i915_private *dev_priv,
struct drm_framebuffer *fb)
@@ -15922,6 +15958,7 @@ intel_user_framebuffer_create(struct drm_device *dev,
static const struct drm_mode_config_funcs intel_mode_funcs = {
.fb_create = intel_user_framebuffer_create,
+ .get_format_info = intel_get_format_info,
.output_poll_changed = intel_fbdev_output_poll_changed,
.atomic_check = intel_atomic_check,
.atomic_commit = intel_atomic_commit,
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index a5890bf44c0a..2926d916f199 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -218,6 +218,9 @@ extern "C" {
*/
#define I915_FORMAT_MOD_Yf_TILED fourcc_mod_code(INTEL, 3)
+#define I915_FORMAT_MOD_Y_TILED_CCS fourcc_mod_code(INTEL, 4)
+#define I915_FORMAT_MOD_Yf_TILED_CCS fourcc_mod_code(INTEL, 5)
+
/*
* Tiled, NV12MT, grouped in 64 (pixels) x 32 (lines) -sized macroblocks
*
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 37/37] drm/i915: Implement .get_format_info() hook for CCS
2016-11-18 19:53 ` [PATCH 37/37] drm/i915: Implement .get_format_info() hook for CCS ville.syrjala
@ 2016-11-18 23:31 ` Ben Widawsky
2016-11-21 14:37 ` Ville Syrjälä
2016-11-21 8:42 ` [Intel-gfx] " Tvrtko Ursulin
1 sibling, 1 reply; 34+ messages in thread
From: Ben Widawsky @ 2016-11-18 23:31 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, Laurent Pinchart, dri-devel
On 16-11-18 21:53:13, Ville Syrjälä wrote:
>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>By providing our own format information for the CCS formats, we should
>be able to make framebuffer_check() do the right thing for the CCS
>surface as well.
>
I was hoping to see that patch as well :-). If you're adding the new fb
modifiers, I think it'd make sense to make it part of this series.
Alternatively, I can take 36, and 37 and make it part of my series, then
integrate that last bit. It's up to you.
>Note that we'll return the same format info for both Y and Yf tiled
>format as that's what happens with the non-CCS Y vs. Yf as well. If
>desired, we could potentially return a unique pointer for each
>pixel_format+tiling+ccs combination, in which case we immediately be
>able to tell if any of that stuff changed by just comparing the
>pointers. But that does sound a bit wasteful space wise.
>
>Cc: Ben Widawsky <ben@bwidawsk.net>
>Cc: intel-gfx@lists.freedesktop.org
>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
I have a comment below however, you can consider it:
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
>---
> drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++++++++++++++++++++++++
> include/uapi/drm/drm_fourcc.h | 3 +++
> 2 files changed, 40 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>index 7b7135be3b9e..de6909770c68 100644
>--- a/drivers/gpu/drm/i915/intel_display.c
>+++ b/drivers/gpu/drm/i915/intel_display.c
>@@ -2488,6 +2488,42 @@ static unsigned int intel_fb_modifier_to_tiling(uint64_t fb_modifier)
> }
> }
>
>+static const struct drm_format_info ccs_formats[] = {
>+ { .format = DRM_FORMAT_XRGB8888, .depth = 24, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
>+ { .format = DRM_FORMAT_XBGR8888, .depth = 24, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
>+ { .format = DRM_FORMAT_ARGB8888, .depth = 32, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
>+ { .format = DRM_FORMAT_ABGR8888, .depth = 32, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
>+};
>+
>+static const struct drm_format_info *
>+lookup_format_info(const struct drm_format_info formats[],
>+ int num_formats, u32 format)
>+{
>+ int i;
>+
>+ for (i = 0; i < num_formats; i++) {
>+ if (formats[i].format == format)
>+ return &formats[i];
>+ }
>+
>+ return NULL;
>+}
>+
>+static const struct drm_format_info *
>+intel_get_format_info(struct drm_device *dev,
>+ const struct drm_mode_fb_cmd2 *cmd)
>+{
>+ switch (cmd->modifier[0]) {
>+ case I915_FORMAT_MOD_Y_TILED_CCS:
>+ case I915_FORMAT_MOD_Yf_TILED_CCS:
>+ return lookup_format_info(ccs_formats,
>+ ARRAY_SIZE(ccs_formats),
>+ cmd->pixel_format);
>+ default:
>+ return NULL;
>+ }
>+}
>+
It sort of seems like somewhat of a waste to provide this if implementations are
allowed to return NULL. It's like saying, "DRM core will check stuff for you if
you provide the info, but you don't have to do it if you don't want to." If
that's the case you may as well provide a driver hook to just do the check, ie.
s/mod_funcs->get_format_info/mode_functs->check_format/
> static int
> intel_fill_fb_info(struct drm_i915_private *dev_priv,
> struct drm_framebuffer *fb)
>@@ -15922,6 +15958,7 @@ intel_user_framebuffer_create(struct drm_device *dev,
>
> static const struct drm_mode_config_funcs intel_mode_funcs = {
> .fb_create = intel_user_framebuffer_create,
>+ .get_format_info = intel_get_format_info,
> .output_poll_changed = intel_fbdev_output_poll_changed,
> .atomic_check = intel_atomic_check,
> .atomic_commit = intel_atomic_commit,
>diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
>index a5890bf44c0a..2926d916f199 100644
>--- a/include/uapi/drm/drm_fourcc.h
>+++ b/include/uapi/drm/drm_fourcc.h
>@@ -218,6 +218,9 @@ extern "C" {
> */
> #define I915_FORMAT_MOD_Yf_TILED fourcc_mod_code(INTEL, 3)
>
>+#define I915_FORMAT_MOD_Y_TILED_CCS fourcc_mod_code(INTEL, 4)
>+#define I915_FORMAT_MOD_Yf_TILED_CCS fourcc_mod_code(INTEL, 5)
>+
> /*
> * Tiled, NV12MT, grouped in 64 (pixels) x 32 (lines) -sized macroblocks
> *
--
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 37/37] drm/i915: Implement .get_format_info() hook for CCS
2016-11-18 23:31 ` Ben Widawsky
@ 2016-11-21 14:37 ` Ville Syrjälä
0 siblings, 0 replies; 34+ messages in thread
From: Ville Syrjälä @ 2016-11-21 14:37 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx, Laurent Pinchart, dri-devel
On Fri, Nov 18, 2016 at 03:31:48PM -0800, Ben Widawsky wrote:
> On 16-11-18 21:53:13, Ville Syrjälä wrote:
> >From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> >By providing our own format information for the CCS formats, we should
> >be able to make framebuffer_check() do the right thing for the CCS
> >surface as well.
> >
>
> I was hoping to see that patch as well :-). If you're adding the new fb
> modifiers, I think it'd make sense to make it part of this series.
> Alternatively, I can take 36, and 37 and make it part of my series, then
> integrate that last bit. It's up to you.
>
> >Note that we'll return the same format info for both Y and Yf tiled
> >format as that's what happens with the non-CCS Y vs. Yf as well. If
> >desired, we could potentially return a unique pointer for each
> >pixel_format+tiling+ccs combination, in which case we immediately be
> >able to tell if any of that stuff changed by just comparing the
> >pointers. But that does sound a bit wasteful space wise.
> >
> >Cc: Ben Widawsky <ben@bwidawsk.net>
> >Cc: intel-gfx@lists.freedesktop.org
> >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> I have a comment below however, you can consider it:
> Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
>
> >---
> > drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++++++++++++++++++++++++
> > include/uapi/drm/drm_fourcc.h | 3 +++
> > 2 files changed, 40 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >index 7b7135be3b9e..de6909770c68 100644
> >--- a/drivers/gpu/drm/i915/intel_display.c
> >+++ b/drivers/gpu/drm/i915/intel_display.c
> >@@ -2488,6 +2488,42 @@ static unsigned int intel_fb_modifier_to_tiling(uint64_t fb_modifier)
> > }
> > }
> >
> >+static const struct drm_format_info ccs_formats[] = {
> >+ { .format = DRM_FORMAT_XRGB8888, .depth = 24, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
> >+ { .format = DRM_FORMAT_XBGR8888, .depth = 24, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
> >+ { .format = DRM_FORMAT_ARGB8888, .depth = 32, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
> >+ { .format = DRM_FORMAT_ABGR8888, .depth = 32, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
> >+};
> >+
> >+static const struct drm_format_info *
> >+lookup_format_info(const struct drm_format_info formats[],
> >+ int num_formats, u32 format)
> >+{
> >+ int i;
> >+
> >+ for (i = 0; i < num_formats; i++) {
> >+ if (formats[i].format == format)
> >+ return &formats[i];
> >+ }
> >+
> >+ return NULL;
> >+}
> >+
> >+static const struct drm_format_info *
> >+intel_get_format_info(struct drm_device *dev,
> >+ const struct drm_mode_fb_cmd2 *cmd)
> >+{
> >+ switch (cmd->modifier[0]) {
> >+ case I915_FORMAT_MOD_Y_TILED_CCS:
> >+ case I915_FORMAT_MOD_Yf_TILED_CCS:
> >+ return lookup_format_info(ccs_formats,
> >+ ARRAY_SIZE(ccs_formats),
> >+ cmd->pixel_format);
> >+ default:
> >+ return NULL;
> >+ }
> >+}
> >+
>
> It sort of seems like somewhat of a waste to provide this if implementations are
> allowed to return NULL. It's like saying, "DRM core will check stuff for you if
> you provide the info, but you don't have to do it if you don't want to."
The core will check the stuff anyway. The NULL just means "I don't have
any special requirements, so the core format info will be sufficient".
> If
> that's the case you may as well provide a driver hook to just do the check, ie.
> s/mod_funcs->get_format_info/mode_functs->check_format/
Drivers already have to do a bunch of checks in .fb_create(). In
addition the core does some basic sanity checks before the driver
even sees the mode_cmd (except for the new .get_format_info() hook
that is). I don't want every driver to have to duplicate all of these
basic sanity checks.
One alternative to this scheme would be have a helper function that
every driver would call in .fb_create() that would do these basic sanity
checks. That way we wouldn't need the extra hook, with a slight risk
that driver would forget to call the helper.
>
> > static int
> > intel_fill_fb_info(struct drm_i915_private *dev_priv,
> > struct drm_framebuffer *fb)
> >@@ -15922,6 +15958,7 @@ intel_user_framebuffer_create(struct drm_device *dev,
> >
> > static const struct drm_mode_config_funcs intel_mode_funcs = {
> > .fb_create = intel_user_framebuffer_create,
> >+ .get_format_info = intel_get_format_info,
> > .output_poll_changed = intel_fbdev_output_poll_changed,
> > .atomic_check = intel_atomic_check,
> > .atomic_commit = intel_atomic_commit,
> >diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> >index a5890bf44c0a..2926d916f199 100644
> >--- a/include/uapi/drm/drm_fourcc.h
> >+++ b/include/uapi/drm/drm_fourcc.h
> >@@ -218,6 +218,9 @@ extern "C" {
> > */
> > #define I915_FORMAT_MOD_Yf_TILED fourcc_mod_code(INTEL, 3)
> >
> >+#define I915_FORMAT_MOD_Y_TILED_CCS fourcc_mod_code(INTEL, 4)
> >+#define I915_FORMAT_MOD_Yf_TILED_CCS fourcc_mod_code(INTEL, 5)
> >+
> > /*
> > * Tiled, NV12MT, grouped in 64 (pixels) x 32 (lines) -sized macroblocks
> > *
>
> --
> Ben Widawsky, Intel Open Source Technology Center
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH 37/37] drm/i915: Implement .get_format_info() hook for CCS
2016-11-18 19:53 ` [PATCH 37/37] drm/i915: Implement .get_format_info() hook for CCS ville.syrjala
2016-11-18 23:31 ` Ben Widawsky
@ 2016-11-21 8:42 ` Tvrtko Ursulin
2016-11-21 13:27 ` Ville Syrjälä
1 sibling, 1 reply; 34+ messages in thread
From: Tvrtko Ursulin @ 2016-11-21 8:42 UTC (permalink / raw)
To: ville.syrjala, dri-devel; +Cc: Ben Widawsky, intel-gfx, Laurent Pinchart
Hi,
On 18/11/2016 19:53, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> By providing our own format information for the CCS formats, we should
> be able to make framebuffer_check() do the right thing for the CCS
> surface as well.
>
> Note that we'll return the same format info for both Y and Yf tiled
> format as that's what happens with the non-CCS Y vs. Yf as well. If
> desired, we could potentially return a unique pointer for each
> pixel_format+tiling+ccs combination, in which case we immediately be
> able to tell if any of that stuff changed by just comparing the
> pointers. But that does sound a bit wasteful space wise.
>
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++++++++++++++++++++++++
> include/uapi/drm/drm_fourcc.h | 3 +++
> 2 files changed, 40 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7b7135be3b9e..de6909770c68 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2488,6 +2488,42 @@ static unsigned int intel_fb_modifier_to_tiling(uint64_t fb_modifier)
> }
> }
>
> +static const struct drm_format_info ccs_formats[] = {
> + { .format = DRM_FORMAT_XRGB8888, .depth = 24, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
> + { .format = DRM_FORMAT_XBGR8888, .depth = 24, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
> + { .format = DRM_FORMAT_ARGB8888, .depth = 32, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
> + { .format = DRM_FORMAT_ABGR8888, .depth = 32, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
> +};
> +
> +static const struct drm_format_info *
> +lookup_format_info(const struct drm_format_info formats[],
> + int num_formats, u32 format)
> +{
> + int i;
> +
> + for (i = 0; i < num_formats; i++) {
> + if (formats[i].format == format)
> + return &formats[i];
> + }
> +
> + return NULL;
> +}
> +
> +static const struct drm_format_info *
> +intel_get_format_info(struct drm_device *dev,
> + const struct drm_mode_fb_cmd2 *cmd)
> +{
> + switch (cmd->modifier[0]) {
> + case I915_FORMAT_MOD_Y_TILED_CCS:
> + case I915_FORMAT_MOD_Yf_TILED_CCS:
> + return lookup_format_info(ccs_formats,
> + ARRAY_SIZE(ccs_formats),
> + cmd->pixel_format);
> + default:
> + return NULL;
> + }
> +}
> +
> static int
> intel_fill_fb_info(struct drm_i915_private *dev_priv,
> struct drm_framebuffer *fb)
> @@ -15922,6 +15958,7 @@ intel_user_framebuffer_create(struct drm_device *dev,
>
> static const struct drm_mode_config_funcs intel_mode_funcs = {
> .fb_create = intel_user_framebuffer_create,
> + .get_format_info = intel_get_format_info,
> .output_poll_changed = intel_fbdev_output_poll_changed,
> .atomic_check = intel_atomic_check,
> .atomic_commit = intel_atomic_commit,
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index a5890bf44c0a..2926d916f199 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -218,6 +218,9 @@ extern "C" {
> */
> #define I915_FORMAT_MOD_Yf_TILED fourcc_mod_code(INTEL, 3)
>
> +#define I915_FORMAT_MOD_Y_TILED_CCS fourcc_mod_code(INTEL, 4)
> +#define I915_FORMAT_MOD_Yf_TILED_CCS fourcc_mod_code(INTEL, 5)
> +
I think when fb modifiers were started the idea was that we would later
partition our vendor bit space for different classes of things and have
helper functions to extract the tiling, etc, from them.
For example have first 3-4 bits represent the tiling, then in this case
one bit for CCS, etc.
Have you considered that when adding these ones, and concluded this
different scheme is better for some reason?
Regards,
Tvrtko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 37/37] drm/i915: Implement .get_format_info() hook for CCS
2016-11-21 8:42 ` [Intel-gfx] " Tvrtko Ursulin
@ 2016-11-21 13:27 ` Ville Syrjälä
2016-11-21 14:04 ` Tvrtko Ursulin
0 siblings, 1 reply; 34+ messages in thread
From: Ville Syrjälä @ 2016-11-21 13:27 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Ben Widawsky, intel-gfx, dri-devel, Laurent Pinchart
On Mon, Nov 21, 2016 at 08:42:13AM +0000, Tvrtko Ursulin wrote:
>
> Hi,
>
> On 18/11/2016 19:53, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > By providing our own format information for the CCS formats, we should
> > be able to make framebuffer_check() do the right thing for the CCS
> > surface as well.
> >
> > Note that we'll return the same format info for both Y and Yf tiled
> > format as that's what happens with the non-CCS Y vs. Yf as well. If
> > desired, we could potentially return a unique pointer for each
> > pixel_format+tiling+ccs combination, in which case we immediately be
> > able to tell if any of that stuff changed by just comparing the
> > pointers. But that does sound a bit wasteful space wise.
> >
> > Cc: Ben Widawsky <ben@bwidawsk.net>
> > Cc: intel-gfx@lists.freedesktop.org
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++++++++++++++++++++++++
> > include/uapi/drm/drm_fourcc.h | 3 +++
> > 2 files changed, 40 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 7b7135be3b9e..de6909770c68 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2488,6 +2488,42 @@ static unsigned int intel_fb_modifier_to_tiling(uint64_t fb_modifier)
> > }
> > }
> >
> > +static const struct drm_format_info ccs_formats[] = {
> > + { .format = DRM_FORMAT_XRGB8888, .depth = 24, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
> > + { .format = DRM_FORMAT_XBGR8888, .depth = 24, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
> > + { .format = DRM_FORMAT_ARGB8888, .depth = 32, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
> > + { .format = DRM_FORMAT_ABGR8888, .depth = 32, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
> > +};
> > +
> > +static const struct drm_format_info *
> > +lookup_format_info(const struct drm_format_info formats[],
> > + int num_formats, u32 format)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < num_formats; i++) {
> > + if (formats[i].format == format)
> > + return &formats[i];
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > +static const struct drm_format_info *
> > +intel_get_format_info(struct drm_device *dev,
> > + const struct drm_mode_fb_cmd2 *cmd)
> > +{
> > + switch (cmd->modifier[0]) {
> > + case I915_FORMAT_MOD_Y_TILED_CCS:
> > + case I915_FORMAT_MOD_Yf_TILED_CCS:
> > + return lookup_format_info(ccs_formats,
> > + ARRAY_SIZE(ccs_formats),
> > + cmd->pixel_format);
> > + default:
> > + return NULL;
> > + }
> > +}
> > +
> > static int
> > intel_fill_fb_info(struct drm_i915_private *dev_priv,
> > struct drm_framebuffer *fb)
> > @@ -15922,6 +15958,7 @@ intel_user_framebuffer_create(struct drm_device *dev,
> >
> > static const struct drm_mode_config_funcs intel_mode_funcs = {
> > .fb_create = intel_user_framebuffer_create,
> > + .get_format_info = intel_get_format_info,
> > .output_poll_changed = intel_fbdev_output_poll_changed,
> > .atomic_check = intel_atomic_check,
> > .atomic_commit = intel_atomic_commit,
> > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> > index a5890bf44c0a..2926d916f199 100644
> > --- a/include/uapi/drm/drm_fourcc.h
> > +++ b/include/uapi/drm/drm_fourcc.h
> > @@ -218,6 +218,9 @@ extern "C" {
> > */
> > #define I915_FORMAT_MOD_Yf_TILED fourcc_mod_code(INTEL, 3)
> >
> > +#define I915_FORMAT_MOD_Y_TILED_CCS fourcc_mod_code(INTEL, 4)
> > +#define I915_FORMAT_MOD_Yf_TILED_CCS fourcc_mod_code(INTEL, 5)
> > +
>
> I think when fb modifiers were started the idea was that we would later
> partition our vendor bit space for different classes of things and have
> helper functions to extract the tiling, etc, from them.
>
> For example have first 3-4 bits represent the tiling, then in this case
> one bit for CCS, etc.
>
> Have you considered that when adding these ones, and concluded this
> different scheme is better for some reason?
I haven't considered anything. And obviously this patch isn't meant
for inclusion as is. I just needed sometime to make it compile.
Generally I don't think adding magic meaning for individual bits for
things like this is a particularly good idea. Every time I've seen a
scheme like that it has eventually turned ugly on account of running
out of bits in one place or another.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 37/37] drm/i915: Implement .get_format_info() hook for CCS
2016-11-21 13:27 ` Ville Syrjälä
@ 2016-11-21 14:04 ` Tvrtko Ursulin
0 siblings, 0 replies; 34+ messages in thread
From: Tvrtko Ursulin @ 2016-11-21 14:04 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Ben Widawsky, intel-gfx, dri-devel, Laurent Pinchart
On 21/11/2016 13:27, Ville Syrjälä wrote:
> On Mon, Nov 21, 2016 at 08:42:13AM +0000, Tvrtko Ursulin wrote:
>>
>> Hi,
>>
>> On 18/11/2016 19:53, ville.syrjala@linux.intel.com wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> By providing our own format information for the CCS formats, we should
>>> be able to make framebuffer_check() do the right thing for the CCS
>>> surface as well.
>>>
>>> Note that we'll return the same format info for both Y and Yf tiled
>>> format as that's what happens with the non-CCS Y vs. Yf as well. If
>>> desired, we could potentially return a unique pointer for each
>>> pixel_format+tiling+ccs combination, in which case we immediately be
>>> able to tell if any of that stuff changed by just comparing the
>>> pointers. But that does sound a bit wasteful space wise.
>>>
>>> Cc: Ben Widawsky <ben@bwidawsk.net>
>>> Cc: intel-gfx@lists.freedesktop.org
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>> drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++++++++++++++++++++++++
>>> include/uapi/drm/drm_fourcc.h | 3 +++
>>> 2 files changed, 40 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index 7b7135be3b9e..de6909770c68 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -2488,6 +2488,42 @@ static unsigned int intel_fb_modifier_to_tiling(uint64_t fb_modifier)
>>> }
>>> }
>>>
>>> +static const struct drm_format_info ccs_formats[] = {
>>> + { .format = DRM_FORMAT_XRGB8888, .depth = 24, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
>>> + { .format = DRM_FORMAT_XBGR8888, .depth = 24, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
>>> + { .format = DRM_FORMAT_ARGB8888, .depth = 32, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
>>> + { .format = DRM_FORMAT_ABGR8888, .depth = 32, .num_planes = 2, .cpp = { 4, 1, }, .hsub = 16, .vsub = 8, },
>>> +};
>>> +
>>> +static const struct drm_format_info *
>>> +lookup_format_info(const struct drm_format_info formats[],
>>> + int num_formats, u32 format)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0; i < num_formats; i++) {
>>> + if (formats[i].format == format)
>>> + return &formats[i];
>>> + }
>>> +
>>> + return NULL;
>>> +}
>>> +
>>> +static const struct drm_format_info *
>>> +intel_get_format_info(struct drm_device *dev,
>>> + const struct drm_mode_fb_cmd2 *cmd)
>>> +{
>>> + switch (cmd->modifier[0]) {
>>> + case I915_FORMAT_MOD_Y_TILED_CCS:
>>> + case I915_FORMAT_MOD_Yf_TILED_CCS:
>>> + return lookup_format_info(ccs_formats,
>>> + ARRAY_SIZE(ccs_formats),
>>> + cmd->pixel_format);
>>> + default:
>>> + return NULL;
>>> + }
>>> +}
>>> +
>>> static int
>>> intel_fill_fb_info(struct drm_i915_private *dev_priv,
>>> struct drm_framebuffer *fb)
>>> @@ -15922,6 +15958,7 @@ intel_user_framebuffer_create(struct drm_device *dev,
>>>
>>> static const struct drm_mode_config_funcs intel_mode_funcs = {
>>> .fb_create = intel_user_framebuffer_create,
>>> + .get_format_info = intel_get_format_info,
>>> .output_poll_changed = intel_fbdev_output_poll_changed,
>>> .atomic_check = intel_atomic_check,
>>> .atomic_commit = intel_atomic_commit,
>>> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
>>> index a5890bf44c0a..2926d916f199 100644
>>> --- a/include/uapi/drm/drm_fourcc.h
>>> +++ b/include/uapi/drm/drm_fourcc.h
>>> @@ -218,6 +218,9 @@ extern "C" {
>>> */
>>> #define I915_FORMAT_MOD_Yf_TILED fourcc_mod_code(INTEL, 3)
>>>
>>> +#define I915_FORMAT_MOD_Y_TILED_CCS fourcc_mod_code(INTEL, 4)
>>> +#define I915_FORMAT_MOD_Yf_TILED_CCS fourcc_mod_code(INTEL, 5)
>>> +
>>
>> I think when fb modifiers were started the idea was that we would later
>> partition our vendor bit space for different classes of things and have
>> helper functions to extract the tiling, etc, from them.
>>
>> For example have first 3-4 bits represent the tiling, then in this case
>> one bit for CCS, etc.
>>
>> Have you considered that when adding these ones, and concluded this
>> different scheme is better for some reason?
>
> I haven't considered anything. And obviously this patch isn't meant
> for inclusion as is. I just needed sometime to make it compile.
No idea on the status of this series. Just noticed new modifiers by
accident and remembered the early discussions.
> Generally I don't think adding magic meaning for individual bits for
> things like this is a particularly good idea. Every time I've seen a
> scheme like that it has eventually turned ugly on account of running
> out of bits in one place or another.
I think in this case it might be much better. You just need one more
feature which intersects with tiling and ccs to make the list not very
manageable.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 00/37] drm: Deduplicate fb format information (v2)
2016-11-18 19:52 [PATCH v2 00/37] drm: Deduplicate fb format information (v2) ville.syrjala
` (7 preceding siblings ...)
2016-11-18 19:53 ` [PATCH 37/37] drm/i915: Implement .get_format_info() hook for CCS ville.syrjala
@ 2016-11-21 11:18 ` Christian König
2016-12-14 21:37 ` Ville Syrjälä
9 siblings, 0 replies; 34+ messages in thread
From: Christian König @ 2016-11-21 11:18 UTC (permalink / raw)
To: ville.syrjala, dri-devel
Cc: Ben Widawsky, Gerd Hoffmann, Paulo Zanoni, Sinclair Yeh,
Liviu Dudau, intel-gfx, Alexey Brodkin, Thomas Hellstrom,
linux-graphics-maintainer, Laurent Pinchart, Mali DP Maintainers,
Alex Deucher, Dave Airlie, Ben Skeggs
Patches #2 and #3 are Reviewed-by: Christian König
<christian.koenig@amd.com>.
The rest is Acked-by: Christian König <christian.koenig@amd.com>.
Regards,
Christian.
Am 18.11.2016 um 20:52 schrieb ville.syrjala@linux.intel.com:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Second installment of my effort to remove the duplicated
> depth/bpp/pixel_format from drm_framebuffer and just use
> struct drm_format_info instead.
>
> I tried to address all of the review feedback, and collect
> up all the r-bs I already got. Thanks for the review, guys.
>
> Changes since the last version are roughly:
> * drm_framebuffer_init() now fails if the fb isn't properly prepared
> * Applied mode cocciry all over to use fb->format more extensively
> * Dropped a few i915 specific patches that were taken care of the
> previous item
> * Took up Laurent's idea that we can just compare the fb->format
> pointers instead of comparing the fb->format->format values
> * Added a new .get_format_info() hooks for drivers to provide custom
> format information + an quick example patch how we'd hook it up
> for i915 render compression support
>
> Link to the previous version:
> https://lists.freedesktop.org/archives/dri-devel/2016-November/124135.html
>
> Entire series is available here:
> git://github.com/vsyrjala/linux.git fb_format_dedup_2
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Cc: Brian Starkey <brian.starkey@arm.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: linux-graphics-maintainer@vmware.com
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Mali DP Maintainers <malidp@foss.arm.com>
> Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
>
> Ville Syrjälä (37):
> drm/i915: Add local 'fb' variables
> drm/radeon: Add local 'fb' variables
> drm/radeon: Use DIV_ROUND_UP()
> drm/mgag200: Add local 'fb' variable
> drm/ast: Add local 'fb' variables
> drm/gma500: Add some local 'fb' variables
> drm/cirrus: Add some local 'fb' variables
> drm/arcpgu: Add local 'fb' variables
> drm/arm: Add local 'fb' variables
> drm/nouveau: Fix crtc->primary->fb vs. drm_fb fail
> drm/nouveau: Add local 'fb' variables
> drm/vmwgfx: Populate fb->dev before drm_framebuffer_init()
> drm: Pass 'dev' to drm_helper_mode_fill_fb_struct()
> drm/vmwgfx: Populate fb->pixel_format
> drm/qxl: Call drm_helper_mode_fill_fb_struct() before
> drm_framebuffer_init()
> drm/virtio: Call drm_helper_mode_fill_fb_struct() before
> drm_framebuffer_init()
> drm/i915: Set fb->dev early on for inherited fbs
> drm: Populate fb->dev from drm_helper_mode_fill_fb_struct()
> drm: Store a pointer to drm_format_info under drm_framebuffer
> drm/vmwgfx: Populate fb->format correctly
> drm/i915: Populate fb->format early for inherited fbs
> drm: Reject fbs w/o format info in drm_framebuffer_init()
> drm: Replace drm_format_num_planes() with fb->format->num_planes
> drm/i915: Eliminate the ugly 'fb?:' constructs from the ilk/skl wm
> code
> drm: Replace drm_format_plane_cpp() with fb->format->cpp[]
> drm/fb_cma_helper: Replace drm_format_info() with fb->format
> drm/nouveau: Use fb->format rather than drm_format_info()
> drm/i915: Store a pointer to the pixel format info for fbc
> drm: Add drm_framebuffer_plane_{width,height}()
> drm/i915: Use drm_framebuffer_plane_{width,height}() where possible
> drm: Nuke fb->depth
> drm: Nuke fb->bits_per_pixel
> drm: Nuke fb->pixel_format
> drm: Replace 'format->format' comparisons to just 'format' comparisons
> drm: Eliminate the useless "non-RGB fb" debug message
> drm: Add mode_config .get_format_info() hook
> drm/i915: Implement .get_format_info() hook for CCS
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 4 +-
> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 6 +-
> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 6 +-
> drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 6 +-
> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 6 +-
> drivers/gpu/drm/arc/arcpgu_crtc.c | 3 +-
> drivers/gpu/drm/arm/hdlcd_crtc.c | 18 ++--
> drivers/gpu/drm/arm/malidp_planes.c | 10 +--
> drivers/gpu/drm/armada/armada_crtc.c | 6 +-
> drivers/gpu/drm/armada/armada_fb.c | 2 +-
> drivers/gpu/drm/armada/armada_fbdev.c | 5 +-
> drivers/gpu/drm/armada/armada_overlay.c | 6 +-
> drivers/gpu/drm/ast/ast_fb.c | 4 +-
> drivers/gpu/drm/ast/ast_main.c | 2 +-
> drivers/gpu/drm/ast/ast_mode.c | 16 ++--
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.c | 2 +-
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 22 ++---
> drivers/gpu/drm/bochs/bochs_fbdev.c | 2 +-
> drivers/gpu/drm/bochs/bochs_mm.c | 2 +-
> drivers/gpu/drm/cirrus/cirrus_fbdev.c | 6 +-
> drivers/gpu/drm/cirrus/cirrus_main.c | 2 +-
> drivers/gpu/drm/cirrus/cirrus_mode.c | 9 +-
> drivers/gpu/drm/drm_atomic.c | 8 +-
> drivers/gpu/drm/drm_crtc.c | 4 +-
> drivers/gpu/drm/drm_crtc_helper.c | 3 +-
> drivers/gpu/drm/drm_fb_cma_helper.c | 13 ++-
> drivers/gpu/drm/drm_fb_helper.c | 10 +--
> drivers/gpu/drm/drm_fourcc.c | 25 ++++++
> drivers/gpu/drm/drm_framebuffer.c | 62 +++++++++++--
> drivers/gpu/drm/drm_modeset_helper.c | 22 +----
> drivers/gpu/drm/drm_plane.c | 6 +-
> drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 6 +-
> drivers/gpu/drm/exynos/exynos7_drm_decon.c | 8 +-
> drivers/gpu/drm/exynos/exynos_drm_fb.c | 2 +-
> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 6 +-
> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 4 +-
> drivers/gpu/drm/exynos/exynos_mixer.c | 12 +--
> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 4 +-
> drivers/gpu/drm/gma500/accel_2d.c | 2 +-
> drivers/gpu/drm/gma500/framebuffer.c | 6 +-
> drivers/gpu/drm/gma500/gma_display.c | 13 +--
> drivers/gpu/drm/gma500/mdfld_intel_display.c | 17 ++--
> drivers/gpu/drm/gma500/oaktrail_crtc.c | 13 +--
> drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 6 +-
> drivers/gpu/drm/i915/i915_debugfs.c | 11 +--
> drivers/gpu/drm/i915/i915_drv.h | 4 +-
> drivers/gpu/drm/i915/intel_atomic_plane.c | 4 +-
> drivers/gpu/drm/i915/intel_display.c | 115 ++++++++++++++++--------
> drivers/gpu/drm/i915/intel_fbc.c | 14 +--
> drivers/gpu/drm/i915/intel_fbdev.c | 10 +--
> drivers/gpu/drm/i915/intel_overlay.c | 26 +++---
> drivers/gpu/drm/i915/intel_pm.c | 67 +++++++-------
> drivers/gpu/drm/i915/intel_sprite.c | 14 +--
> drivers/gpu/drm/imx/ipuv3-plane.c | 40 ++++-----
> drivers/gpu/drm/mediatek/mtk_drm_fb.c | 2 +-
> drivers/gpu/drm/mediatek/mtk_drm_plane.c | 4 +-
> drivers/gpu/drm/mgag200/mgag200_fb.c | 4 +-
> drivers/gpu/drm/mgag200/mgag200_main.c | 2 +-
> drivers/gpu/drm/mgag200/mgag200_mode.c | 23 ++---
> drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c | 2 +-
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 4 +-
> drivers/gpu/drm/msm/msm_fb.c | 12 +--
> drivers/gpu/drm/msm/msm_fbdev.c | 2 +-
> drivers/gpu/drm/nouveau/dispnv04/crtc.c | 17 ++--
> drivers/gpu/drm/nouveau/dispnv04/dfp.c | 3 +-
> drivers/gpu/drm/nouveau/dispnv04/overlay.c | 8 +-
> drivers/gpu/drm/nouveau/nouveau_display.c | 4 +-
> drivers/gpu/drm/nouveau/nouveau_fbcon.c | 3 +-
> drivers/gpu/drm/nouveau/nv50_display.c | 14 ++-
> drivers/gpu/drm/omapdrm/omap_fb.c | 12 +--
> drivers/gpu/drm/omapdrm/omap_fbdev.c | 2 +-
> drivers/gpu/drm/qxl/qxl_display.c | 2 +-
> drivers/gpu/drm/qxl/qxl_draw.c | 2 +-
> drivers/gpu/drm/qxl/qxl_fb.c | 5 +-
> drivers/gpu/drm/radeon/atombios_crtc.c | 19 ++--
> drivers/gpu/drm/radeon/r100.c | 10 ++-
> drivers/gpu/drm/radeon/radeon_display.c | 8 +-
> drivers/gpu/drm/radeon/radeon_fb.c | 4 +-
> drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 16 ++--
> drivers/gpu/drm/rcar-du/rcar_du_plane.c | 4 +-
> drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 4 +-
> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 +-
> drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 5 +-
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 22 ++---
> drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 6 +-
> drivers/gpu/drm/shmobile/shmob_drm_plane.c | 4 +-
> drivers/gpu/drm/sti/sti_gdp.c | 10 +--
> drivers/gpu/drm/sti/sti_hqvdp.c | 2 +-
> drivers/gpu/drm/sun4i/sun4i_backend.c | 5 +-
> drivers/gpu/drm/tegra/dc.c | 8 +-
> drivers/gpu/drm/tegra/drm.c | 5 +-
> drivers/gpu/drm/tegra/fb.c | 6 +-
> drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 4 +-
> drivers/gpu/drm/tilcdc/tilcdc_plane.c | 4 +-
> drivers/gpu/drm/udl/udl_fb.c | 6 +-
> drivers/gpu/drm/vc4/vc4_plane.c | 8 +-
> drivers/gpu/drm/virtio/virtgpu_display.c | 3 +-
> drivers/gpu/drm/virtio/virtgpu_fb.c | 4 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 13 +--
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 29 ++++--
> drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 5 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 4 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 +-
> drivers/gpu/drm/zte/zx_plane.c | 4 +-
> include/drm/drm_fourcc.h | 6 ++
> include/drm/drm_framebuffer.h | 27 +++---
> include/drm/drm_mode_config.h | 15 ++++
> include/drm/drm_modeset_helper.h | 3 +-
> include/uapi/drm/drm_fourcc.h | 3 +
> 110 files changed, 641 insertions(+), 470 deletions(-)
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v2 00/37] drm: Deduplicate fb format information (v2)
2016-11-18 19:52 [PATCH v2 00/37] drm: Deduplicate fb format information (v2) ville.syrjala
` (8 preceding siblings ...)
2016-11-21 11:18 ` [PATCH v2 00/37] drm: Deduplicate fb format information (v2) Christian König
@ 2016-12-14 21:37 ` Ville Syrjälä
2016-12-15 13:41 ` Ville Syrjälä
9 siblings, 1 reply; 34+ messages in thread
From: Ville Syrjälä @ 2016-12-14 21:37 UTC (permalink / raw)
To: dri-devel
Cc: Ben Widawsky, Gerd Hoffmann, Paulo Zanoni, Sinclair Yeh,
Liviu Dudau, intel-gfx, Alexey Brodkin, Thomas Hellstrom,
Christian König, linux-graphics-maintainer, Laurent Pinchart,
Mali DP Maintainers, Alex Deucher, Dave Airlie, Ben Skeggs
On Fri, Nov 18, 2016 at 09:52:36PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Second installment of my effort to remove the duplicated
> depth/bpp/pixel_format from drm_framebuffer and just use
> struct drm_format_info instead.
>
> I tried to address all of the review feedback, and collect
> up all the r-bs I already got. Thanks for the review, guys.
>
> Changes since the last version are roughly:
> * drm_framebuffer_init() now fails if the fb isn't properly prepared
> * Applied mode cocciry all over to use fb->format more extensively
> * Dropped a few i915 specific patches that were taken care of the
> previous item
> * Took up Laurent's idea that we can just compare the fb->format
> pointers instead of comparing the fb->format->format values
> * Added a new .get_format_info() hooks for drivers to provide custom
> format information + an quick example patch how we'd hook it up
> for i915 render compression support
>
> Link to the previous version:
> https://lists.freedesktop.org/archives/dri-devel/2016-November/124135.html
>
> Entire series is available here:
> git://github.com/vsyrjala/linux.git fb_format_dedup_2
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Cc: Brian Starkey <brian.starkey@arm.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: linux-graphics-maintainer@vmware.com
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Mali DP Maintainers <malidp@foss.arm.com>
> Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
>
> Ville Syrjälä (37):
> drm/i915: Add local 'fb' variables
> drm/radeon: Add local 'fb' variables
> drm/radeon: Use DIV_ROUND_UP()
> drm/mgag200: Add local 'fb' variable
> drm/ast: Add local 'fb' variables
> drm/gma500: Add some local 'fb' variables
> drm/cirrus: Add some local 'fb' variables
> drm/arcpgu: Add local 'fb' variables
> drm/arm: Add local 'fb' variables
> drm/nouveau: Fix crtc->primary->fb vs. drm_fb fail
> drm/nouveau: Add local 'fb' variables
I've pushed up to here to drm-misc-next. Thanks for the reviews.
I re-ran spatch to regenerate some of the later patches as there had
been a bit of churn in the code. I've reposted the changed patches,
and if no one screams I'll be pushing the rest tomorrowish.
> drm/vmwgfx: Populate fb->dev before drm_framebuffer_init()
> drm: Pass 'dev' to drm_helper_mode_fill_fb_struct()
> drm/vmwgfx: Populate fb->pixel_format
> drm/qxl: Call drm_helper_mode_fill_fb_struct() before
> drm_framebuffer_init()
> drm/virtio: Call drm_helper_mode_fill_fb_struct() before
> drm_framebuffer_init()
> drm/i915: Set fb->dev early on for inherited fbs
> drm: Populate fb->dev from drm_helper_mode_fill_fb_struct()
> drm: Store a pointer to drm_format_info under drm_framebuffer
> drm/vmwgfx: Populate fb->format correctly
> drm/i915: Populate fb->format early for inherited fbs
> drm: Reject fbs w/o format info in drm_framebuffer_init()
> drm: Replace drm_format_num_planes() with fb->format->num_planes
> drm/i915: Eliminate the ugly 'fb?:' constructs from the ilk/skl wm
> code
> drm: Replace drm_format_plane_cpp() with fb->format->cpp[]
> drm/fb_cma_helper: Replace drm_format_info() with fb->format
> drm/nouveau: Use fb->format rather than drm_format_info()
> drm/i915: Store a pointer to the pixel format info for fbc
> drm: Add drm_framebuffer_plane_{width,height}()
> drm/i915: Use drm_framebuffer_plane_{width,height}() where possible
> drm: Nuke fb->depth
> drm: Nuke fb->bits_per_pixel
> drm: Nuke fb->pixel_format
> drm: Replace 'format->format' comparisons to just 'format' comparisons
> drm: Eliminate the useless "non-RGB fb" debug message
> drm: Add mode_config .get_format_info() hook
> drm/i915: Implement .get_format_info() hook for CCS
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 4 +-
> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 6 +-
> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 6 +-
> drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 6 +-
> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 6 +-
> drivers/gpu/drm/arc/arcpgu_crtc.c | 3 +-
> drivers/gpu/drm/arm/hdlcd_crtc.c | 18 ++--
> drivers/gpu/drm/arm/malidp_planes.c | 10 +--
> drivers/gpu/drm/armada/armada_crtc.c | 6 +-
> drivers/gpu/drm/armada/armada_fb.c | 2 +-
> drivers/gpu/drm/armada/armada_fbdev.c | 5 +-
> drivers/gpu/drm/armada/armada_overlay.c | 6 +-
> drivers/gpu/drm/ast/ast_fb.c | 4 +-
> drivers/gpu/drm/ast/ast_main.c | 2 +-
> drivers/gpu/drm/ast/ast_mode.c | 16 ++--
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.c | 2 +-
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 22 ++---
> drivers/gpu/drm/bochs/bochs_fbdev.c | 2 +-
> drivers/gpu/drm/bochs/bochs_mm.c | 2 +-
> drivers/gpu/drm/cirrus/cirrus_fbdev.c | 6 +-
> drivers/gpu/drm/cirrus/cirrus_main.c | 2 +-
> drivers/gpu/drm/cirrus/cirrus_mode.c | 9 +-
> drivers/gpu/drm/drm_atomic.c | 8 +-
> drivers/gpu/drm/drm_crtc.c | 4 +-
> drivers/gpu/drm/drm_crtc_helper.c | 3 +-
> drivers/gpu/drm/drm_fb_cma_helper.c | 13 ++-
> drivers/gpu/drm/drm_fb_helper.c | 10 +--
> drivers/gpu/drm/drm_fourcc.c | 25 ++++++
> drivers/gpu/drm/drm_framebuffer.c | 62 +++++++++++--
> drivers/gpu/drm/drm_modeset_helper.c | 22 +----
> drivers/gpu/drm/drm_plane.c | 6 +-
> drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 6 +-
> drivers/gpu/drm/exynos/exynos7_drm_decon.c | 8 +-
> drivers/gpu/drm/exynos/exynos_drm_fb.c | 2 +-
> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 6 +-
> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 4 +-
> drivers/gpu/drm/exynos/exynos_mixer.c | 12 +--
> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 4 +-
> drivers/gpu/drm/gma500/accel_2d.c | 2 +-
> drivers/gpu/drm/gma500/framebuffer.c | 6 +-
> drivers/gpu/drm/gma500/gma_display.c | 13 +--
> drivers/gpu/drm/gma500/mdfld_intel_display.c | 17 ++--
> drivers/gpu/drm/gma500/oaktrail_crtc.c | 13 +--
> drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 6 +-
> drivers/gpu/drm/i915/i915_debugfs.c | 11 +--
> drivers/gpu/drm/i915/i915_drv.h | 4 +-
> drivers/gpu/drm/i915/intel_atomic_plane.c | 4 +-
> drivers/gpu/drm/i915/intel_display.c | 115 ++++++++++++++++--------
> drivers/gpu/drm/i915/intel_fbc.c | 14 +--
> drivers/gpu/drm/i915/intel_fbdev.c | 10 +--
> drivers/gpu/drm/i915/intel_overlay.c | 26 +++---
> drivers/gpu/drm/i915/intel_pm.c | 67 +++++++-------
> drivers/gpu/drm/i915/intel_sprite.c | 14 +--
> drivers/gpu/drm/imx/ipuv3-plane.c | 40 ++++-----
> drivers/gpu/drm/mediatek/mtk_drm_fb.c | 2 +-
> drivers/gpu/drm/mediatek/mtk_drm_plane.c | 4 +-
> drivers/gpu/drm/mgag200/mgag200_fb.c | 4 +-
> drivers/gpu/drm/mgag200/mgag200_main.c | 2 +-
> drivers/gpu/drm/mgag200/mgag200_mode.c | 23 ++---
> drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c | 2 +-
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 4 +-
> drivers/gpu/drm/msm/msm_fb.c | 12 +--
> drivers/gpu/drm/msm/msm_fbdev.c | 2 +-
> drivers/gpu/drm/nouveau/dispnv04/crtc.c | 17 ++--
> drivers/gpu/drm/nouveau/dispnv04/dfp.c | 3 +-
> drivers/gpu/drm/nouveau/dispnv04/overlay.c | 8 +-
> drivers/gpu/drm/nouveau/nouveau_display.c | 4 +-
> drivers/gpu/drm/nouveau/nouveau_fbcon.c | 3 +-
> drivers/gpu/drm/nouveau/nv50_display.c | 14 ++-
> drivers/gpu/drm/omapdrm/omap_fb.c | 12 +--
> drivers/gpu/drm/omapdrm/omap_fbdev.c | 2 +-
> drivers/gpu/drm/qxl/qxl_display.c | 2 +-
> drivers/gpu/drm/qxl/qxl_draw.c | 2 +-
> drivers/gpu/drm/qxl/qxl_fb.c | 5 +-
> drivers/gpu/drm/radeon/atombios_crtc.c | 19 ++--
> drivers/gpu/drm/radeon/r100.c | 10 ++-
> drivers/gpu/drm/radeon/radeon_display.c | 8 +-
> drivers/gpu/drm/radeon/radeon_fb.c | 4 +-
> drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 16 ++--
> drivers/gpu/drm/rcar-du/rcar_du_plane.c | 4 +-
> drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 4 +-
> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 +-
> drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 5 +-
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 22 ++---
> drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 6 +-
> drivers/gpu/drm/shmobile/shmob_drm_plane.c | 4 +-
> drivers/gpu/drm/sti/sti_gdp.c | 10 +--
> drivers/gpu/drm/sti/sti_hqvdp.c | 2 +-
> drivers/gpu/drm/sun4i/sun4i_backend.c | 5 +-
> drivers/gpu/drm/tegra/dc.c | 8 +-
> drivers/gpu/drm/tegra/drm.c | 5 +-
> drivers/gpu/drm/tegra/fb.c | 6 +-
> drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 4 +-
> drivers/gpu/drm/tilcdc/tilcdc_plane.c | 4 +-
> drivers/gpu/drm/udl/udl_fb.c | 6 +-
> drivers/gpu/drm/vc4/vc4_plane.c | 8 +-
> drivers/gpu/drm/virtio/virtgpu_display.c | 3 +-
> drivers/gpu/drm/virtio/virtgpu_fb.c | 4 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 13 +--
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 29 ++++--
> drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 5 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 4 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 +-
> drivers/gpu/drm/zte/zx_plane.c | 4 +-
> include/drm/drm_fourcc.h | 6 ++
> include/drm/drm_framebuffer.h | 27 +++---
> include/drm/drm_mode_config.h | 15 ++++
> include/drm/drm_modeset_helper.h | 3 +-
> include/uapi/drm/drm_fourcc.h | 3 +
> 110 files changed, 641 insertions(+), 470 deletions(-)
>
> --
> 2.7.4
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v2 00/37] drm: Deduplicate fb format information (v2)
2016-12-14 21:37 ` Ville Syrjälä
@ 2016-12-15 13:41 ` Ville Syrjälä
0 siblings, 0 replies; 34+ messages in thread
From: Ville Syrjälä @ 2016-12-15 13:41 UTC (permalink / raw)
To: dri-devel
Cc: Ben Widawsky, Paulo Zanoni, Sinclair Yeh, Alexey Brodkin,
intel-gfx, Liviu Dudau, Thomas Hellstrom, Ben Skeggs,
linux-graphics-maintainer, Gerd Hoffmann, Dave Airlie,
Alex Deucher, Mali DP Maintainers, Christian König,
Laurent Pinchart
On Wed, Dec 14, 2016 at 11:37:46PM +0200, Ville Syrjälä wrote:
> On Fri, Nov 18, 2016 at 09:52:36PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Second installment of my effort to remove the duplicated
> > depth/bpp/pixel_format from drm_framebuffer and just use
> > struct drm_format_info instead.
> >
> > I tried to address all of the review feedback, and collect
> > up all the r-bs I already got. Thanks for the review, guys.
> >
> > Changes since the last version are roughly:
> > * drm_framebuffer_init() now fails if the fb isn't properly prepared
> > * Applied mode cocciry all over to use fb->format more extensively
> > * Dropped a few i915 specific patches that were taken care of the
> > previous item
> > * Took up Laurent's idea that we can just compare the fb->format
> > pointers instead of comparing the fb->format->format values
> > * Added a new .get_format_info() hooks for drivers to provide custom
> > format information + an quick example patch how we'd hook it up
> > for i915 render compression support
> >
> > Link to the previous version:
> > https://lists.freedesktop.org/archives/dri-devel/2016-November/124135.html
> >
> > Entire series is available here:
> > git://github.com/vsyrjala/linux.git fb_format_dedup_2
> >
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: Alexey Brodkin <abrodkin@synopsys.com>
> > Cc: Ben Skeggs <bskeggs@redhat.com>
> > Cc: Ben Widawsky <ben@bwidawsk.net>
> > Cc: Brian Starkey <brian.starkey@arm.com>
> > Cc: "Christian König" <christian.koenig@amd.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: intel-gfx@lists.freedesktop.org
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: linux-graphics-maintainer@vmware.com
> > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > Cc: Mali DP Maintainers <malidp@foss.arm.com>
> > Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Sinclair Yeh <syeh@vmware.com>
> > Cc: Thomas Hellstrom <thellstrom@vmware.com>
> >
> > Ville Syrjälä (37):
> > drm/i915: Add local 'fb' variables
> > drm/radeon: Add local 'fb' variables
> > drm/radeon: Use DIV_ROUND_UP()
> > drm/mgag200: Add local 'fb' variable
> > drm/ast: Add local 'fb' variables
> > drm/gma500: Add some local 'fb' variables
> > drm/cirrus: Add some local 'fb' variables
> > drm/arcpgu: Add local 'fb' variables
> > drm/arm: Add local 'fb' variables
> > drm/nouveau: Fix crtc->primary->fb vs. drm_fb fail
> > drm/nouveau: Add local 'fb' variables
>
> I've pushed up to here to drm-misc-next. Thanks for the reviews.
>
> I re-ran spatch to regenerate some of the later patches as there had
> been a bit of churn in the code. I've reposted the changed patches,
> and if no one screams I'll be pushing the rest tomorrowish.
>
> > drm/vmwgfx: Populate fb->dev before drm_framebuffer_init()
> > drm: Pass 'dev' to drm_helper_mode_fill_fb_struct()
> > drm/vmwgfx: Populate fb->pixel_format
> > drm/qxl: Call drm_helper_mode_fill_fb_struct() before
> > drm_framebuffer_init()
> > drm/virtio: Call drm_helper_mode_fill_fb_struct() before
> > drm_framebuffer_init()
> > drm/i915: Set fb->dev early on for inherited fbs
> > drm: Populate fb->dev from drm_helper_mode_fill_fb_struct()
> > drm: Store a pointer to drm_format_info under drm_framebuffer
> > drm/vmwgfx: Populate fb->format correctly
> > drm/i915: Populate fb->format early for inherited fbs
> > drm: Reject fbs w/o format info in drm_framebuffer_init()
> > drm: Replace drm_format_num_planes() with fb->format->num_planes
> > drm/i915: Eliminate the ugly 'fb?:' constructs from the ilk/skl wm
> > code
> > drm: Replace drm_format_plane_cpp() with fb->format->cpp[]
> > drm/fb_cma_helper: Replace drm_format_info() with fb->format
> > drm/nouveau: Use fb->format rather than drm_format_info()
> > drm/i915: Store a pointer to the pixel format info for fbc
> > drm: Add drm_framebuffer_plane_{width,height}()
> > drm/i915: Use drm_framebuffer_plane_{width,height}() where possible
> > drm: Nuke fb->depth
> > drm: Nuke fb->bits_per_pixel
> > drm: Nuke fb->pixel_format
> > drm: Replace 'format->format' comparisons to just 'format' comparisons
> > drm: Eliminate the useless "non-RGB fb" debug message
And I've just pushed up to here (minus the vmvgfx patches which dropped
out due to Daniel's earlier refactorin).
> > drm: Add mode_config .get_format_info() hook
> > drm/i915: Implement .get_format_info() hook for CCS
I'll hang on to these until we get the i915 CCS thing into shape.
Thanks for the reviews everyone.
> >
> > drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 2 +-
> > drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 4 +-
> > drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 6 +-
> > drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 6 +-
> > drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 6 +-
> > drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 6 +-
> > drivers/gpu/drm/arc/arcpgu_crtc.c | 3 +-
> > drivers/gpu/drm/arm/hdlcd_crtc.c | 18 ++--
> > drivers/gpu/drm/arm/malidp_planes.c | 10 +--
> > drivers/gpu/drm/armada/armada_crtc.c | 6 +-
> > drivers/gpu/drm/armada/armada_fb.c | 2 +-
> > drivers/gpu/drm/armada/armada_fbdev.c | 5 +-
> > drivers/gpu/drm/armada/armada_overlay.c | 6 +-
> > drivers/gpu/drm/ast/ast_fb.c | 4 +-
> > drivers/gpu/drm/ast/ast_main.c | 2 +-
> > drivers/gpu/drm/ast/ast_mode.c | 16 ++--
> > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_layer.c | 2 +-
> > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 22 ++---
> > drivers/gpu/drm/bochs/bochs_fbdev.c | 2 +-
> > drivers/gpu/drm/bochs/bochs_mm.c | 2 +-
> > drivers/gpu/drm/cirrus/cirrus_fbdev.c | 6 +-
> > drivers/gpu/drm/cirrus/cirrus_main.c | 2 +-
> > drivers/gpu/drm/cirrus/cirrus_mode.c | 9 +-
> > drivers/gpu/drm/drm_atomic.c | 8 +-
> > drivers/gpu/drm/drm_crtc.c | 4 +-
> > drivers/gpu/drm/drm_crtc_helper.c | 3 +-
> > drivers/gpu/drm/drm_fb_cma_helper.c | 13 ++-
> > drivers/gpu/drm/drm_fb_helper.c | 10 +--
> > drivers/gpu/drm/drm_fourcc.c | 25 ++++++
> > drivers/gpu/drm/drm_framebuffer.c | 62 +++++++++++--
> > drivers/gpu/drm/drm_modeset_helper.c | 22 +----
> > drivers/gpu/drm/drm_plane.c | 6 +-
> > drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 6 +-
> > drivers/gpu/drm/exynos/exynos7_drm_decon.c | 8 +-
> > drivers/gpu/drm/exynos/exynos_drm_fb.c | 2 +-
> > drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 6 +-
> > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 4 +-
> > drivers/gpu/drm/exynos/exynos_mixer.c | 12 +--
> > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 4 +-
> > drivers/gpu/drm/gma500/accel_2d.c | 2 +-
> > drivers/gpu/drm/gma500/framebuffer.c | 6 +-
> > drivers/gpu/drm/gma500/gma_display.c | 13 +--
> > drivers/gpu/drm/gma500/mdfld_intel_display.c | 17 ++--
> > drivers/gpu/drm/gma500/oaktrail_crtc.c | 13 +--
> > drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 6 +-
> > drivers/gpu/drm/i915/i915_debugfs.c | 11 +--
> > drivers/gpu/drm/i915/i915_drv.h | 4 +-
> > drivers/gpu/drm/i915/intel_atomic_plane.c | 4 +-
> > drivers/gpu/drm/i915/intel_display.c | 115 ++++++++++++++++--------
> > drivers/gpu/drm/i915/intel_fbc.c | 14 +--
> > drivers/gpu/drm/i915/intel_fbdev.c | 10 +--
> > drivers/gpu/drm/i915/intel_overlay.c | 26 +++---
> > drivers/gpu/drm/i915/intel_pm.c | 67 +++++++-------
> > drivers/gpu/drm/i915/intel_sprite.c | 14 +--
> > drivers/gpu/drm/imx/ipuv3-plane.c | 40 ++++-----
> > drivers/gpu/drm/mediatek/mtk_drm_fb.c | 2 +-
> > drivers/gpu/drm/mediatek/mtk_drm_plane.c | 4 +-
> > drivers/gpu/drm/mgag200/mgag200_fb.c | 4 +-
> > drivers/gpu/drm/mgag200/mgag200_main.c | 2 +-
> > drivers/gpu/drm/mgag200/mgag200_mode.c | 23 ++---
> > drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c | 2 +-
> > drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 4 +-
> > drivers/gpu/drm/msm/msm_fb.c | 12 +--
> > drivers/gpu/drm/msm/msm_fbdev.c | 2 +-
> > drivers/gpu/drm/nouveau/dispnv04/crtc.c | 17 ++--
> > drivers/gpu/drm/nouveau/dispnv04/dfp.c | 3 +-
> > drivers/gpu/drm/nouveau/dispnv04/overlay.c | 8 +-
> > drivers/gpu/drm/nouveau/nouveau_display.c | 4 +-
> > drivers/gpu/drm/nouveau/nouveau_fbcon.c | 3 +-
> > drivers/gpu/drm/nouveau/nv50_display.c | 14 ++-
> > drivers/gpu/drm/omapdrm/omap_fb.c | 12 +--
> > drivers/gpu/drm/omapdrm/omap_fbdev.c | 2 +-
> > drivers/gpu/drm/qxl/qxl_display.c | 2 +-
> > drivers/gpu/drm/qxl/qxl_draw.c | 2 +-
> > drivers/gpu/drm/qxl/qxl_fb.c | 5 +-
> > drivers/gpu/drm/radeon/atombios_crtc.c | 19 ++--
> > drivers/gpu/drm/radeon/r100.c | 10 ++-
> > drivers/gpu/drm/radeon/radeon_display.c | 8 +-
> > drivers/gpu/drm/radeon/radeon_fb.c | 4 +-
> > drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 16 ++--
> > drivers/gpu/drm/rcar-du/rcar_du_plane.c | 4 +-
> > drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 4 +-
> > drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 +-
> > drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 5 +-
> > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 22 ++---
> > drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 6 +-
> > drivers/gpu/drm/shmobile/shmob_drm_plane.c | 4 +-
> > drivers/gpu/drm/sti/sti_gdp.c | 10 +--
> > drivers/gpu/drm/sti/sti_hqvdp.c | 2 +-
> > drivers/gpu/drm/sun4i/sun4i_backend.c | 5 +-
> > drivers/gpu/drm/tegra/dc.c | 8 +-
> > drivers/gpu/drm/tegra/drm.c | 5 +-
> > drivers/gpu/drm/tegra/fb.c | 6 +-
> > drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 4 +-
> > drivers/gpu/drm/tilcdc/tilcdc_plane.c | 4 +-
> > drivers/gpu/drm/udl/udl_fb.c | 6 +-
> > drivers/gpu/drm/vc4/vc4_plane.c | 8 +-
> > drivers/gpu/drm/virtio/virtgpu_display.c | 3 +-
> > drivers/gpu/drm/virtio/virtgpu_fb.c | 4 +-
> > drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 13 +--
> > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 29 ++++--
> > drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 5 +-
> > drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 4 +-
> > drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 +-
> > drivers/gpu/drm/zte/zx_plane.c | 4 +-
> > include/drm/drm_fourcc.h | 6 ++
> > include/drm/drm_framebuffer.h | 27 +++---
> > include/drm/drm_mode_config.h | 15 ++++
> > include/drm/drm_modeset_helper.h | 3 +-
> > include/uapi/drm/drm_fourcc.h | 3 +
> > 110 files changed, 641 insertions(+), 470 deletions(-)
> >
> > --
> > 2.7.4
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 34+ messages in thread