On Tue, May 02, 2017 at 10:14:26PM -0700, Ben Widawsky wrote:
> v2: A minor addition from Daniel
>
> Cc: Daniel Stone <daniels@xxxxxxxxxxxxx>
You are *really* pushing your luck by not Cc-ing *any* of the maintainers of
the drivers you touch. You do want reviews, don't you?
> Signed-off-by: Ben Widawsky <ben@xxxxxxxxxxxx>
> ---
> drivers/gpu/drm/arc/arcpgu_crtc.c | 1 +
> drivers/gpu/drm/arm/hdlcd_crtc.c | 1 +
> drivers/gpu/drm/arm/malidp_planes.c | 2 +-
> drivers/gpu/drm/armada/armada_crtc.c | 1 +
> drivers/gpu/drm/armada/armada_overlay.c | 1 +
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 4 +++-
> drivers/gpu/drm/drm_modeset_helper.c | 1 +
> drivers/gpu/drm/drm_plane.c | 32 ++++++++++++++++++++++++-
> drivers/gpu/drm/drm_simple_kms_helper.c | 3 +++
> drivers/gpu/drm/exynos/exynos_drm_plane.c | 2 +-
> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 2 +-
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 1 +
> drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 2 +-
> drivers/gpu/drm/i915/intel_display.c | 7 +++++-
> drivers/gpu/drm/i915/intel_sprite.c | 4 ++--
> drivers/gpu/drm/imx/ipuv3-plane.c | 4 ++--
> drivers/gpu/drm/mediatek/mtk_drm_plane.c | 2 +-
> drivers/gpu/drm/meson/meson_plane.c | 1 +
> drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c | 2 +-
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 4 ++--
> drivers/gpu/drm/mxsfb/mxsfb_drv.c | 2 +-
> drivers/gpu/drm/nouveau/nv50_display.c | 5 ++--
> drivers/gpu/drm/omapdrm/omap_plane.c | 3 ++-
> drivers/gpu/drm/qxl/qxl_display.c | 2 +-
> 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_vop.c | 4 ++--
> drivers/gpu/drm/sti/sti_cursor.c | 2 +-
> drivers/gpu/drm/sti/sti_gdp.c | 2 +-
> drivers/gpu/drm/sti/sti_hqvdp.c | 2 +-
> drivers/gpu/drm/sun4i/sun4i_layer.c | 2 +-
> drivers/gpu/drm/tegra/dc.c | 12 +++++-----
> drivers/gpu/drm/vc4/vc4_plane.c | 2 +-
> drivers/gpu/drm/virtio/virtgpu_plane.c | 2 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 4 ++--
> drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 4 ++--
> drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 4 ++--
> drivers/gpu/drm/zte/zx_plane.c | 2 +-
> include/drm/drm_plane.h | 21 +++++++++++++++-
> include/drm/drm_simple_kms_helper.h | 1 +
> include/uapi/drm/drm_fourcc.h | 11 +++++++++
> 41 files changed, 126 insertions(+), 46 deletions(-)
I wish there was a way to sort the non-driver-specific changes out of this patch and
put the at the beginning or at the end of the patchset. That way one does not have to
hunt through the file for the relevant changes in the API.
How about introducing a new function called drm_universal_plane_mod_init() that has
the new parameter, a second patch that moves the relevant/all drivers to the new function
and (if all drivers were converted) a third patch to rename drm_universal_plane_mod_init()
back to drm_universal_plane_init()? I know it is more churn, but I'm struggling to figure
out why all the drivers have to pass a NULL here. Either they care about modifiers or
they don't, in which case a separate function would make things more obvious.
Finally, the questions I should've started with: why the change? What are you trying to
achieve? It is not very clear from the commit message at all.
>
> diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
> index ad9a95916f1f..cd8a24c7c67d 100644
> --- a/drivers/gpu/drm/arc/arcpgu_crtc.c
> +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
> @@ -218,6 +218,7 @@ static struct drm_plane *arc_pgu_plane_init(struct drm_device *drm)
>
> ret = drm_universal_plane_init(drm, plane, 0xff, &arc_pgu_plane_funcs,
> formats, ARRAY_SIZE(formats),
> + NULL,
> DRM_PLANE_TYPE_PRIMARY, NULL);
> if (ret)
> return ERR_PTR(ret);
> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
> index 798a3cc480a2..0caa03ae8708 100644
> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> @@ -303,6 +303,7 @@ static struct drm_plane *hdlcd_plane_init(struct drm_device *drm)
>
> ret = drm_universal_plane_init(drm, plane, 0xff, &hdlcd_plane_funcs,
> formats, ARRAY_SIZE(formats),
> + NULL,
> DRM_PLANE_TYPE_PRIMARY, NULL);
> if (ret) {
> devm_kfree(drm->dev, plane);
> diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
> index 814fda23cead..b156610c68a5 100644
> --- a/drivers/gpu/drm/arm/malidp_planes.c
> +++ b/drivers/gpu/drm/arm/malidp_planes.c
> @@ -400,7 +400,7 @@ int malidp_de_planes_init(struct drm_device *drm)
> DRM_PLANE_TYPE_OVERLAY;
> ret = drm_universal_plane_init(drm, &plane->base, crtcs,
> &malidp_de_plane_funcs, formats,
> - n, plane_type, NULL);
> + n, NULL, plane_type, NULL);
It would be nice if you can be consistent with the change. Either add the NULL on a new line
or keep updating the line where the new parameter would fit. Did you do this change manually
or used coccinelle?
> if (ret < 0)
> goto cleanup;
>
> diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
> index 4fe19fde84f9..ea48ef88f0e4 100644
> --- a/drivers/gpu/drm/armada/armada_crtc.c
> +++ b/drivers/gpu/drm/armada/armada_crtc.c
> @@ -1269,6 +1269,7 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev,
> &armada_primary_plane_funcs,
> armada_primary_formats,
> ARRAY_SIZE(armada_primary_formats),
> + NULL,
> DRM_PLANE_TYPE_PRIMARY, NULL);
> if (ret) {
> kfree(primary);
> diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c
> index 424e465ff407..0054144287ae 100644
> --- a/drivers/gpu/drm/armada/armada_overlay.c
> +++ b/drivers/gpu/drm/armada/armada_overlay.c
> @@ -460,6 +460,7 @@ int armada_overlay_plane_create(struct drm_device *dev, unsigned long crtcs)
> &armada_ovl_plane_funcs,
> armada_ovl_formats,
> ARRAY_SIZE(armada_ovl_formats),
> + NULL,
> DRM_PLANE_TYPE_OVERLAY, NULL);
> if (ret) {
> kfree(dplane);
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index 29cc10d053eb..b5c6cf2d8c36 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -1058,7 +1058,9 @@ static int atmel_hlcdc_plane_create(struct drm_device *dev,
> ret = drm_universal_plane_init(dev, &plane->base, 0,
> &layer_plane_funcs,
> desc->formats->formats,
> - desc->formats->nformats, type, NULL);
> + desc->formats->nformats,
> + NULL,
> + type, NULL);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
> index 2b33825f2f93..9cb1eede0b4d 100644
> --- a/drivers/gpu/drm/drm_modeset_helper.c
> +++ b/drivers/gpu/drm/drm_modeset_helper.c
> @@ -124,6 +124,7 @@ static struct drm_plane *create_primary_plane(struct drm_device *dev)
> &drm_primary_helper_funcs,
> safe_modeset_formats,
> ARRAY_SIZE(safe_modeset_formats),
> + NULL,
> DRM_PLANE_TYPE_PRIMARY, NULL);
> if (ret) {
> kfree(primary);
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index fedd4d60d9cd..286e183891e5 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -70,6 +70,7 @@ static unsigned int drm_num_planes(struct drm_device *dev)
> * @funcs: callbacks for the new plane
> * @formats: array of supported formats (DRM_FORMAT\_\*)
> * @format_count: number of elements in @formats
> + * @format_modifiers: array of struct drm_format modifiers terminated by INVALID
You actually called the thing DRM_FORMAT_MOD_INVALID. Make sure the documentation correctly
reflects that.
> * @type: type of plane (overlay, primary, cursor)
> * @name: printf style format string for the plane name, or NULL for default name
> *
> @@ -82,10 +83,12 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
> uint32_t possible_crtcs,
> const struct drm_plane_funcs *funcs,
> const uint32_t *formats, unsigned int format_count,
> + const uint64_t *format_modifiers,
> enum drm_plane_type type,
> const char *name, ...)
> {
> struct drm_mode_config *config = &dev->mode_config;
> + unsigned int format_modifier_count = 0;
> int ret;
>
> ret = drm_mode_object_add(dev, &plane->base, DRM_MODE_OBJECT_PLANE);
> @@ -105,6 +108,28 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
> return -ENOMEM;
> }
>
> + /* First driver to need more than 64 formats needs to fix this */
> + if (WARN_ON(format_count > 64))
> + return -EINVAL;
What is the link between format_count and format modifiers? Why are you introducing
this artificial restriction on the format_count? Also, there doesn't seem to be any
check if format_modifier_count == format_count. What happens when they don't match?
> +
> + if (format_modifiers) {
> + const uint64_t *temp_modifiers = format_modifiers;
> + while (*temp_modifiers++ != DRM_FORMAT_MOD_INVALID)
> + format_modifier_count++;
> + }
> +
> + plane->modifier_count = format_modifier_count;
Why do you need to remember this? It is not used anywhere else!
> + plane->modifiers = kmalloc_array(format_modifier_count,
> + sizeof(format_modifiers[0]),
> + GFP_KERNEL);
> +
> + if (format_modifier_count && !plane->modifiers) {
> + DRM_DEBUG_KMS("out of memory when allocating plane\n");
> + kfree(plane->format_types);
> + drm_mode_object_unregister(dev, &plane->base);
> + return -ENOMEM;
> + }
> +
> if (name) {
> va_list ap;
>
> @@ -117,12 +142,15 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
> }
> if (!plane->name) {
> kfree(plane->format_types);
> + kfree(plane->modifiers);
> drm_mode_object_unregister(dev, &plane->base);
> return -ENOMEM;
> }
>
> memcpy(plane->format_types, formats, format_count * sizeof(uint32_t));
> plane->format_count = format_count;
> + memcpy(plane->modifiers, format_modifiers,
> + format_modifier_count * sizeof(format_modifiers[0]));
Don't you need to check that format_modifiers != NULL before memcpy?
> plane->possible_crtcs = possible_crtcs;
> plane->type = type;
>
> @@ -205,7 +233,8 @@ int drm_plane_init(struct drm_device *dev, struct drm_plane *plane,
>
> type = is_primary ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY;
> return drm_universal_plane_init(dev, plane, possible_crtcs, funcs,
> - formats, format_count, type, NULL);
> + formats, format_count,
> + NULL, type, NULL);
> }
> EXPORT_SYMBOL(drm_plane_init);
>
> @@ -224,6 +253,7 @@ void drm_plane_cleanup(struct drm_plane *plane)
> drm_modeset_lock_fini(&plane->mutex);
>
> kfree(plane->format_types);
> + kfree(plane->modifiers);
> drm_mode_object_unregister(dev, &plane->base);
>
> BUG_ON(list_empty(&plane->head));
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> index e084f9f8ca66..949f0ab0e267 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -193,6 +193,7 @@ EXPORT_SYMBOL(drm_simple_display_pipe_attach_bridge);
> * @funcs: callbacks for the display pipe (optional)
> * @formats: array of supported formats (DRM_FORMAT\_\*)
> * @format_count: number of elements in @formats
> + * @format_modifiers: array of formats modifiers
> * @connector: connector to attach and register (optional)
> *
> * Sets up a display pipeline which consist of a really simple
> @@ -213,6 +214,7 @@ int drm_simple_display_pipe_init(struct drm_device *dev,
> struct drm_simple_display_pipe *pipe,
> const struct drm_simple_display_pipe_funcs *funcs,
> const uint32_t *formats, unsigned int format_count,
> + const uint64_t *format_modifiers,
> struct drm_connector *connector)
> {
> struct drm_encoder *encoder = &pipe->encoder;
> @@ -227,6 +229,7 @@ int drm_simple_display_pipe_init(struct drm_device *dev,
> ret = drm_universal_plane_init(dev, plane, 0,
> &drm_simple_kms_plane_funcs,
> formats, format_count,
> + format_modifiers,
> DRM_PLANE_TYPE_PRIMARY, NULL);
> if (ret)
> return ret;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> index c2f17f30afab..75d4928dd196 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> @@ -284,7 +284,7 @@ int exynos_plane_init(struct drm_device *dev,
> &exynos_plane_funcs,
> config->pixel_formats,
> config->num_pixel_formats,
> - config->type, NULL);
> + NULL, config->type, NULL);
> if (err) {
> DRM_ERROR("failed to initialize plane\n");
> return err;
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> index 0a20723aa6e1..9554b245746e 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> @@ -224,7 +224,7 @@ struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev)
> &fsl_dcu_drm_plane_funcs,
> fsl_dcu_drm_plane_formats,
> ARRAY_SIZE(fsl_dcu_drm_plane_formats),
> - DRM_PLANE_TYPE_PRIMARY, NULL);
> + NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> if (ret) {
> kfree(primary);
> primary = NULL;
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> index 59542bddc980..339e914cbaa3 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> @@ -181,6 +181,7 @@ static struct drm_plane *hibmc_plane_init(struct hibmc_drm_private *priv)
> ret = drm_universal_plane_init(dev, plane, 1, &hibmc_plane_funcs,
> channel_formats1,
> ARRAY_SIZE(channel_formats1),
> + NULL,
> DRM_PLANE_TYPE_PRIMARY,
> NULL);
> if (ret) {
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> index c96c228a9898..1acb8af12246 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> @@ -909,7 +909,7 @@ static int ade_plane_init(struct drm_device *dev, struct ade_plane *aplane,
> return ret;
>
> ret = drm_universal_plane_init(dev, &aplane->base, 1, &ade_plane_funcs,
> - fmts, fmts_cnt, type, NULL);
> + fmts, fmts_cnt, NULL, type, NULL);
> if (ret) {
> DRM_ERROR("fail to init plane, ch=%d\n", aplane->ch);
> return ret;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3617927af269..d52bd05a017d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13516,6 +13516,8 @@ intel_legacy_cursor_update(struct drm_plane *plane,
> src_x, src_y, src_w, src_h, ctx);
> }
>
> +
> +
Extra blank lines not needed here.
> static const struct drm_plane_funcs intel_cursor_plane_funcs = {
> .update_plane = intel_legacy_cursor_update,
> .disable_plane = drm_atomic_helper_disable_plane,
> @@ -13594,18 +13596,21 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
> ret = drm_universal_plane_init(&dev_priv->drm, &primary->base,
> 0, &intel_plane_funcs,
> intel_primary_formats, num_formats,
> + NULL,
> DRM_PLANE_TYPE_PRIMARY,
> "plane 1%c", pipe_name(pipe));
> else if (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv))
> ret = drm_universal_plane_init(&dev_priv->drm, &primary->base,
> 0, &intel_plane_funcs,
> intel_primary_formats, num_formats,
> + NULL,
> DRM_PLANE_TYPE_PRIMARY,
> "primary %c", pipe_name(pipe));
> else
> ret = drm_universal_plane_init(&dev_priv->drm, &primary->base,
> 0, &intel_plane_funcs,
> intel_primary_formats, num_formats,
> + NULL,
> DRM_PLANE_TYPE_PRIMARY,
> "plane %c", plane_name(primary->plane));
> if (ret)
> @@ -13776,7 +13781,7 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
> 0, &intel_cursor_plane_funcs,
> intel_cursor_formats,
> ARRAY_SIZE(intel_cursor_formats),
> - DRM_PLANE_TYPE_CURSOR,
> + NULL, DRM_PLANE_TYPE_CURSOR,
> "cursor %c", pipe_name(pipe));
> if (ret)
> goto fail;
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index f7d431427115..053802dd08c1 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1165,13 +1165,13 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
> ret = drm_universal_plane_init(&dev_priv->drm, &intel_plane->base,
> possible_crtcs, &intel_plane_funcs,
> plane_formats, num_plane_formats,
> - DRM_PLANE_TYPE_OVERLAY,
> + NULL, DRM_PLANE_TYPE_OVERLAY,
> "plane %d%c", plane + 2, pipe_name(pipe));
> else
> ret = drm_universal_plane_init(&dev_priv->drm, &intel_plane->base,
> possible_crtcs, &intel_plane_funcs,
> plane_formats, num_plane_formats,
> - DRM_PLANE_TYPE_OVERLAY,
> + NULL, DRM_PLANE_TYPE_OVERLAY,
> "sprite %c", sprite_name(pipe, plane));
> if (ret)
> goto fail;
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
> index d63e853a0300..6c708c3b1cdc 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -718,8 +718,8 @@ struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu,
>
> ret = drm_universal_plane_init(dev, &ipu_plane->base, possible_crtcs,
> &ipu_plane_funcs, ipu_plane_formats,
> - ARRAY_SIZE(ipu_plane_formats), type,
> - NULL);
> + ARRAY_SIZE(ipu_plane_formats),
> + NULL, type, NULL);
> if (ret) {
> DRM_ERROR("failed to initialize plane\n");
> kfree(ipu_plane);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> index e405e89ed5e5..bec6d14dd070 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> @@ -173,7 +173,7 @@ int mtk_plane_init(struct drm_device *dev, struct drm_plane *plane,
>
> err = drm_universal_plane_init(dev, plane, possible_crtcs,
> &mtk_plane_funcs, formats,
> - ARRAY_SIZE(formats), type, NULL);
> + ARRAY_SIZE(formats), NULL, type, NULL);
> if (err) {
> DRM_ERROR("failed to initialize plane\n");
> return err;
> diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
> index a32d3b6e2e12..17e96fa47868 100644
> --- a/drivers/gpu/drm/meson/meson_plane.c
> +++ b/drivers/gpu/drm/meson/meson_plane.c
> @@ -223,6 +223,7 @@ int meson_plane_create(struct meson_drm *priv)
> &meson_plane_funcs,
> supported_drm_formats,
> ARRAY_SIZE(supported_drm_formats),
> + NULL,
> DRM_PLANE_TYPE_PRIMARY, "meson_primary_plane");
>
> drm_plane_helper_add(plane, &meson_plane_helper_funcs);
> diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
> index 53619d07677e..8f3417e45d4e 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
> @@ -398,7 +398,7 @@ struct drm_plane *mdp4_plane_init(struct drm_device *dev,
> type = private_plane ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY;
> ret = drm_universal_plane_init(dev, plane, 0xff, &mdp4_plane_funcs,
> mdp4_plane->formats, mdp4_plane->nformats,
> - type, NULL);
> + NULL, type, NULL);
> if (ret)
> goto fail;
>
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> index a38c5fe6cc19..a815bc0e78d1 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> @@ -1140,12 +1140,12 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev,
> ret = drm_universal_plane_init(dev, plane, 0xff,
> &mdp5_cursor_plane_funcs,
> mdp5_plane->formats, mdp5_plane->nformats,
> - type, NULL);
> + NULL, type, NULL);
> else
> ret = drm_universal_plane_init(dev, plane, 0xff,
> &mdp5_plane_funcs,
> mdp5_plane->formats, mdp5_plane->nformats,
> - type, NULL);
> + NULL, type, NULL);
> if (ret)
> goto fail;
>
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> index d1b9c34c7c00..3ee3784a54f4 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> @@ -190,7 +190,7 @@ static int mxsfb_load(struct drm_device *drm, unsigned long flags)
> }
>
> ret = drm_simple_display_pipe_init(drm, &mxsfb->pipe, &mxsfb_funcs,
> - mxsfb_formats, ARRAY_SIZE(mxsfb_formats),
> + mxsfb_formats, ARRAY_SIZE(mxsfb_formats), NULL, 0,
> &mxsfb->connector);
> if (ret < 0) {
> dev_err(drm->dev, "Cannot setup simple display pipe\n");
> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
> index 0e58537352fe..dde71be463f8 100644
> --- a/drivers/gpu/drm/nouveau/nv50_display.c
> +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> @@ -1084,8 +1084,9 @@ nv50_wndw_ctor(const struct nv50_wndw_func *func, struct drm_device *dev,
> wndw->func = func;
> wndw->dmac = dmac;
>
> - ret = drm_universal_plane_init(dev, &wndw->plane, 0, &nv50_wndw, format,
> - nformat, type, "%s-%d", name, index);
> + ret = drm_universal_plane_init(dev, &wndw->plane, 0, &nv50_wndw,
> + format, nformat, NULL,
> + type, "%s-%d", name, index);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
> index 9168154d749e..9b8341d77468 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -370,7 +370,8 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
>
> ret = drm_universal_plane_init(dev, plane, possible_crtcs,
> &omap_plane_funcs, omap_plane->formats,
> - omap_plane->nformats, type, NULL);
> + omap_plane->nformats,
> + NULL, type, NULL);
> if (ret < 0)
> goto error;
>
> diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
> index 058340a002c2..fcf1d2034449 100644
> --- a/drivers/gpu/drm/qxl/qxl_display.c
> +++ b/drivers/gpu/drm/qxl/qxl_display.c
> @@ -788,7 +788,7 @@ static struct drm_plane *qxl_create_plane(struct qxl_device *qdev,
>
> err = drm_universal_plane_init(&qdev->ddev, plane, possible_crtcs,
> funcs, formats, num_formats,
> - type, NULL);
> + NULL, type, NULL);
> if (err)
> goto free_plane;
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> index dcde6288da6c..2b02eccbfb70 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> @@ -743,8 +743,8 @@ int rcar_du_planes_init(struct rcar_du_group *rgrp)
>
> ret = drm_universal_plane_init(rcdu->ddev, &plane->plane, crtcs,
> &rcar_du_plane_funcs, formats,
> - ARRAY_SIZE(formats), type,
> - NULL);
> + ARRAY_SIZE(formats),
> + NULL, type, NULL);
> if (ret < 0)
> return ret;
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> index b0ff304ce3dc..e0c054f9b57a 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> @@ -368,8 +368,8 @@ int rcar_du_vsp_init(struct rcar_du_vsp *vsp)
> 1 << vsp->index,
> &rcar_du_vsp_plane_funcs,
> formats_kms,
> - ARRAY_SIZE(formats_kms), type,
> - NULL);
> + ARRAY_SIZE(formats_kms),
> + NULL, type, NULL);
> if (ret < 0)
> return ret;
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 3f7a82d1e095..d0cf57de9afc 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1280,7 +1280,7 @@ static int vop_create_crtc(struct vop *vop)
> 0, &vop_plane_funcs,
> win_data->phy->data_formats,
> win_data->phy->nformats,
> - win_data->type, NULL);
> + NULL, win_data->type, NULL);
> if (ret) {
> DRM_DEV_ERROR(vop->dev, "failed to init plane %d\n",
> ret);
> @@ -1319,7 +1319,7 @@ static int vop_create_crtc(struct vop *vop)
> &vop_plane_funcs,
> win_data->phy->data_formats,
> win_data->phy->nformats,
> - win_data->type, NULL);
> + NULL, win_data->type, NULL);
> if (ret) {
> DRM_DEV_ERROR(vop->dev, "failed to init overlay %d\n",
> ret);
> diff --git a/drivers/gpu/drm/sti/sti_cursor.c b/drivers/gpu/drm/sti/sti_cursor.c
> index cca75bddb9ad..97c25e204bf4 100644
> --- a/drivers/gpu/drm/sti/sti_cursor.c
> +++ b/drivers/gpu/drm/sti/sti_cursor.c
> @@ -393,7 +393,7 @@ struct drm_plane *sti_cursor_create(struct drm_device *drm_dev,
> &sti_cursor_plane_helpers_funcs,
> cursor_supported_formats,
> ARRAY_SIZE(cursor_supported_formats),
> - DRM_PLANE_TYPE_CURSOR, NULL);
> + NULL, DRM_PLANE_TYPE_CURSOR, NULL);
> if (res) {
> DRM_ERROR("Failed to initialize universal plane\n");
> goto err_plane;
> diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
> index 88f16cdf6a4b..70391603c12d 100644
> --- a/drivers/gpu/drm/sti/sti_gdp.c
> +++ b/drivers/gpu/drm/sti/sti_gdp.c
> @@ -932,7 +932,7 @@ struct drm_plane *sti_gdp_create(struct drm_device *drm_dev,
> &sti_gdp_plane_helpers_funcs,
> gdp_supported_formats,
> ARRAY_SIZE(gdp_supported_formats),
> - type, NULL);
> + NULL, type, NULL);
> if (res) {
> DRM_ERROR("Failed to initialize universal plane\n");
> goto err;
> diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
> index 66f843148ef7..9a1ff352820d 100644
> --- a/drivers/gpu/drm/sti/sti_hqvdp.c
> +++ b/drivers/gpu/drm/sti/sti_hqvdp.c
> @@ -1296,7 +1296,7 @@ static struct drm_plane *sti_hqvdp_create(struct drm_device *drm_dev,
> &sti_hqvdp_plane_helpers_funcs,
> hqvdp_supported_formats,
> ARRAY_SIZE(hqvdp_supported_formats),
> - DRM_PLANE_TYPE_OVERLAY, NULL);
> + NULL, DRM_PLANE_TYPE_OVERLAY, NULL);
> if (res) {
> DRM_ERROR("Failed to initialize universal plane\n");
> return NULL;
> diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
> index f26bde5b9117..2d9f8d01ac2e 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
> @@ -115,7 +115,7 @@ static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm,
> ret = drm_universal_plane_init(drm, &layer->plane, 0,
> &sun4i_backend_layer_funcs,
> plane->formats, plane->nformats,
> - plane->type, NULL);
> + NULL, plane->type, NULL);
> if (ret) {
> dev_err(drm->dev, "Couldn't initialize layer\n");
> return ERR_PTR(ret);
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index 95b373f739f2..0380edd054ac 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -655,8 +655,8 @@ static struct drm_plane *tegra_dc_primary_plane_create(struct drm_device *drm,
>
> err = drm_universal_plane_init(drm, &plane->base, possible_crtcs,
> &tegra_primary_plane_funcs, formats,
> - num_formats, DRM_PLANE_TYPE_PRIMARY,
> - NULL);
> + num_formats, NULL,
> + DRM_PLANE_TYPE_PRIMARY, NULL);
> if (err < 0) {
> kfree(plane);
> return ERR_PTR(err);
> @@ -821,8 +821,8 @@ static struct drm_plane *tegra_dc_cursor_plane_create(struct drm_device *drm,
>
> err = drm_universal_plane_init(drm, &plane->base, 1 << dc->pipe,
> &tegra_cursor_plane_funcs, formats,
> - num_formats, DRM_PLANE_TYPE_CURSOR,
> - NULL);
> + num_formats, NULL,
> + DRM_PLANE_TYPE_CURSOR, NULL);
> if (err < 0) {
> kfree(plane);
> return ERR_PTR(err);
> @@ -883,8 +883,8 @@ static struct drm_plane *tegra_dc_overlay_plane_create(struct drm_device *drm,
>
> err = drm_universal_plane_init(drm, &plane->base, 1 << dc->pipe,
> &tegra_overlay_plane_funcs, formats,
> - num_formats, DRM_PLANE_TYPE_OVERLAY,
> - NULL);
> + num_formats, NULL,
> + DRM_PLANE_TYPE_OVERLAY, NULL);
> if (err < 0) {
> kfree(plane);
> return ERR_PTR(err);
> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
> index d34cd5393a9b..74b5e7afd6e9 100644
> --- a/drivers/gpu/drm/vc4/vc4_plane.c
> +++ b/drivers/gpu/drm/vc4/vc4_plane.c
> @@ -861,7 +861,7 @@ struct drm_plane *vc4_plane_init(struct drm_device *dev,
> ret = drm_universal_plane_init(dev, plane, 0,
> &vc4_plane_funcs,
> formats, num_formats,
> - type, NULL);
> + NULL, type, NULL);
>
> drm_plane_helper_add(plane, &vc4_plane_helper_funcs);
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
> index adcdbd0abef6..71ba455af915 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> @@ -298,7 +298,7 @@ struct drm_plane *virtio_gpu_plane_init(struct virtio_gpu_device *vgdev,
> ret = drm_universal_plane_init(dev, plane, 1 << index,
> &virtio_gpu_plane_funcs,
> formats, nformats,
> - type, NULL);
> + NULL, type, NULL);
> if (ret)
> goto err_plane_init;
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> index d3987bcf53f8..6f0bf6f9c6f8 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> @@ -439,7 +439,7 @@ static int vmw_ldu_init(struct vmw_private *dev_priv, unsigned unit)
> 0, &vmw_ldu_plane_funcs,
> vmw_primary_plane_formats,
> ARRAY_SIZE(vmw_primary_plane_formats),
> - DRM_PLANE_TYPE_PRIMARY, NULL);
> + NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> if (ret) {
> DRM_ERROR("Failed to initialize primary plane");
> goto err_free;
> @@ -454,7 +454,7 @@ static int vmw_ldu_init(struct vmw_private *dev_priv, unsigned unit)
> 0, &vmw_ldu_cursor_funcs,
> vmw_cursor_plane_formats,
> ARRAY_SIZE(vmw_cursor_plane_formats),
> - DRM_PLANE_TYPE_CURSOR, NULL);
> + NULL, DRM_PLANE_TYPE_CURSOR, NULL);
> if (ret) {
> DRM_ERROR("Failed to initialize cursor plane");
> drm_plane_cleanup(&ldu->base.primary);
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> index 8d7dc9def7c2..42780c9cb90f 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> @@ -622,7 +622,7 @@ static int vmw_sou_init(struct vmw_private *dev_priv, unsigned unit)
> 0, &vmw_sou_plane_funcs,
> vmw_primary_plane_formats,
> ARRAY_SIZE(vmw_primary_plane_formats),
> - DRM_PLANE_TYPE_PRIMARY, NULL);
> + NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> if (ret) {
> DRM_ERROR("Failed to initialize primary plane");
> goto err_free;
> @@ -637,7 +637,7 @@ static int vmw_sou_init(struct vmw_private *dev_priv, unsigned unit)
> 0, &vmw_sou_cursor_funcs,
> vmw_cursor_plane_formats,
> ARRAY_SIZE(vmw_cursor_plane_formats),
> - DRM_PLANE_TYPE_CURSOR, NULL);
> + NULL, DRM_PLANE_TYPE_CURSOR, NULL);
> if (ret) {
> DRM_ERROR("Failed to initialize cursor plane");
> drm_plane_cleanup(&sou->base.primary);
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> index bad31bdf09b6..f18fb966ce9c 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> @@ -1456,7 +1456,7 @@ static int vmw_stdu_init(struct vmw_private *dev_priv, unsigned unit)
> 0, &vmw_stdu_plane_funcs,
> vmw_primary_plane_formats,
> ARRAY_SIZE(vmw_primary_plane_formats),
> - DRM_PLANE_TYPE_PRIMARY, NULL);
> + NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> if (ret) {
> DRM_ERROR("Failed to initialize primary plane");
> goto err_free;
> @@ -1471,7 +1471,7 @@ static int vmw_stdu_init(struct vmw_private *dev_priv, unsigned unit)
> 0, &vmw_stdu_cursor_funcs,
> vmw_cursor_plane_formats,
> ARRAY_SIZE(vmw_cursor_plane_formats),
> - DRM_PLANE_TYPE_CURSOR, NULL);
> + NULL, DRM_PLANE_TYPE_CURSOR, NULL);
> if (ret) {
> DRM_ERROR("Failed to initialize cursor plane");
> drm_plane_cleanup(&stdu->base.primary);
> diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
> index d646ac931663..ea29fee01f7d 100644
> --- a/drivers/gpu/drm/zte/zx_plane.c
> +++ b/drivers/gpu/drm/zte/zx_plane.c
> @@ -539,7 +539,7 @@ int zx_plane_init(struct drm_device *drm, struct zx_plane *zplane,
>
> ret = drm_universal_plane_init(drm, plane, VOU_CRTC_MASK,
> &zx_plane_funcs, formats, format_count,
> - type, NULL);
> + NULL, type, NULL);
> if (ret) {
> DRM_DEV_ERROR(dev, "failed to init universal plane: %d\n", ret);
> return ret;
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 9ab3e7044812..4011d5527f40 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -392,6 +392,20 @@ struct drm_plane_funcs {
> */
> void (*atomic_print_state)(struct drm_printer *p,
> const struct drm_plane_state *state);
> +
> + /**
> + * @format_mod_supported:
> + *
> + * This optional hook is used for the DRM to determine if the given
> + * format/modifier combination is valid for the plane. This allows the
> + * DRM to generate the correct format bitmask (which formats apply to
> + * which modifier).
> + *
> + * True if the given modifier is valid for that format on the plane.
> + * False otherwise.
> + */
> + bool (*format_mod_supported)(struct drm_plane *plane, uint32_t format,
> + uint64_t modifier);
This additional function is not used by anything in this patch. Why include it here?
> };
>
> /**
> @@ -487,6 +501,10 @@ struct drm_plane {
> unsigned int format_count;
> bool format_default;
>
> + uint32_t *formats;
Who's using formats in this patch?
> + uint64_t *modifiers;
> + unsigned int modifier_count;
> +
> struct drm_crtc *crtc;
> struct drm_framebuffer *fb;
>
> @@ -527,13 +545,14 @@ struct drm_plane {
>
> #define obj_to_plane(x) container_of(x, struct drm_plane, base)
>
> -__printf(8, 9)
> +__printf(9, 10)
> int drm_universal_plane_init(struct drm_device *dev,
> struct drm_plane *plane,
> uint32_t possible_crtcs,
> const struct drm_plane_funcs *funcs,
> const uint32_t *formats,
> unsigned int format_count,
> + const uint64_t *format_modifiers,
> enum drm_plane_type type,
> const char *name, ...);
> int drm_plane_init(struct drm_device *dev,
> diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
> index 2d36538e4a17..6d9adbb46293 100644
> --- a/include/drm/drm_simple_kms_helper.h
> +++ b/include/drm/drm_simple_kms_helper.h
> @@ -122,6 +122,7 @@ int drm_simple_display_pipe_init(struct drm_device *dev,
> struct drm_simple_display_pipe *pipe,
> const struct drm_simple_display_pipe_funcs *funcs,
> const uint32_t *formats, unsigned int format_count,
> + const uint64_t *format_modifiers,
> struct drm_connector *connector);
>
> #endif /* __LINUX_DRM_SIMPLE_KMS_HELPER_H */
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 995c8f9c692f..ddabbeeebdec 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -184,6 +184,8 @@ extern "C" {
> #define DRM_FORMAT_MOD_VENDOR_VIVANTE 0x06
> /* add more to the end as needed */
>
> +#define DRM_FORMAT_RESERVED ((1ULL << 56) - 1)
> +
> #define fourcc_mod_code(vendor, val) \
> ((((__u64)DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | (val & 0x00ffffffffffffffULL))
>
> @@ -196,6 +198,15 @@ extern "C" {
> */
>
> /*
> + * Invalid Modifier
> + *
> + * This modifier can be used as a sentinel to terminate list, or to initialize a
> + * variable with an invalid modifier. It might also be used to report an error
> + * back to userspace for certain APIs.
> + */
> +#define DRM_FORMAT_MOD_INVALID fourcc_mod_code(NONE, DRM_FORMAT_RESERVED)
> +
> +/*
> * Linear Layout
> *
> * Just plain linear layout. Note that this is different from no specifying any
> --
> 2.12.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
I understand you want to add support for modifiers into planes and it is something that
I'm interested in, but I don't think this patch is doing it well enough.
Best regards,
Liviu
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
