* [PATCH] drm/simple-kms: Standardize arguments for callbacks
@ 2019-10-22 15:55 Daniel Vetter
2019-10-22 17:16 ` Thomas Zimmermann
` (5 more replies)
0 siblings, 6 replies; 22+ messages in thread
From: Daniel Vetter @ 2019-10-22 15:55 UTC (permalink / raw)
To: DRI Development
Cc: Daniel Vetter, virtualization, Gerd Hoffmann, Thomas Zimmermann,
Daniel Vetter, Emil Velikov
Passing the wrong type feels icky, everywhere else we use the pipe as
the first parameter. Spotted while discussing patches with Thomas
Zimmermann.
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Noralf Trønnes <noralf@tronnes.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Emil Velikov <emil.velikov@collabora.com>
Cc: virtualization@lists.linux-foundation.org
Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/cirrus/cirrus.c | 2 +-
drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
drivers/gpu/drm/pl111/pl111_display.c | 4 ++--
include/drm/drm_simple_kms_helper.h | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
index 7d08d067e1a4..248c9f765c45 100644
--- a/drivers/gpu/drm/cirrus/cirrus.c
+++ b/drivers/gpu/drm/cirrus/cirrus.c
@@ -390,7 +390,7 @@ static int cirrus_conn_init(struct cirrus_device *cirrus)
/* ------------------------------------------------------------------ */
/* cirrus (simple) display pipe */
-static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_crtc *crtc,
+static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
const struct drm_display_mode *mode)
{
if (cirrus_check_size(mode->hdisplay, mode->vdisplay, NULL) < 0)
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index 046055719245..15fb516ae2d8 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -43,7 +43,7 @@ drm_simple_kms_crtc_mode_valid(struct drm_crtc *crtc,
/* Anything goes */
return MODE_OK;
- return pipe->funcs->mode_valid(crtc, mode);
+ return pipe->funcs->mode_valid(pipe, mode);
}
static int drm_simple_kms_crtc_check(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 024771a4083e..703ddc803c55 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -48,10 +48,10 @@ irqreturn_t pl111_irq(int irq, void *data)
}
static enum drm_mode_status
-pl111_mode_valid(struct drm_crtc *crtc,
+pl111_mode_valid(struct drm_simple_display_pipe *pipe,
const struct drm_display_mode *mode)
{
- struct drm_device *drm = crtc->dev;
+ struct drm_device *drm = pipe->crtc.dev;
struct pl111_drm_dev_private *priv = drm->dev_private;
u32 cpp = priv->variant->fb_bpp / 8;
u64 bw;
diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
index 4d89cd0a60db..15afee9cf049 100644
--- a/include/drm/drm_simple_kms_helper.h
+++ b/include/drm/drm_simple_kms_helper.h
@@ -49,7 +49,7 @@ struct drm_simple_display_pipe_funcs {
*
* drm_mode_status Enum
*/
- enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc,
+ enum drm_mode_status (*mode_valid)(struct drm_simple_display_pipe *pipe,
const struct drm_display_mode *mode);
/**
--
2.23.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/simple-kms: Standardize arguments for callbacks
2019-10-22 15:55 [PATCH] drm/simple-kms: Standardize arguments for callbacks Daniel Vetter
@ 2019-10-22 17:16 ` Thomas Zimmermann
2019-10-22 17:16 ` Thomas Zimmermann
` (4 subsequent siblings)
5 siblings, 0 replies; 22+ messages in thread
From: Thomas Zimmermann @ 2019-10-22 17:16 UTC (permalink / raw)
To: Daniel Vetter, DRI Development
Cc: Linus Walleij, virtualization, Eric Anholt, Noralf Trønnes,
Daniel Vetter, Emil Velikov
[-- Attachment #1.1.1: Type: text/plain, Size: 4432 bytes --]
Hi,
there are two types of callbacks in struct
drm_simple_display_pipe_funcs: functions that are genuine to simple KMS,
and functions that are merely forwarded from another structure (crtc,
plane, etc).
In the former category are enable(), disable(), check(), and update().
Those should probably receive a simple display pipe as their first argument.
In the latter category are mode_valid(), prepare_fb(), cleanup_fb(),
enable_vblank(), and disable_vblank(). IMHO those functions should
receive a pointer to the original structure as their first argument.
This type provides the context in which the operations make sense. (Even
their documentation already refers to the original structure.)
I admit that not all are as shareable as prepare_fb() and enable_fb().
But what else than boiler-plate wrappers do we get from simple display
pipe here?
Best regards
Thomas
Am 22.10.19 um 17:55 schrieb Daniel Vetter:
> Passing the wrong type feels icky, everywhere else we use the pipe as
> the first parameter. Spotted while discussing patches with Thomas
> Zimmermann.
>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Noralf Trønnes <noralf@tronnes.org>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Emil Velikov <emil.velikov@collabora.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> drivers/gpu/drm/cirrus/cirrus.c | 2 +-
> drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
> drivers/gpu/drm/pl111/pl111_display.c | 4 ++--
> include/drm/drm_simple_kms_helper.h | 2 +-
> 4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
> index 7d08d067e1a4..248c9f765c45 100644
> --- a/drivers/gpu/drm/cirrus/cirrus.c
> +++ b/drivers/gpu/drm/cirrus/cirrus.c
> @@ -390,7 +390,7 @@ static int cirrus_conn_init(struct cirrus_device *cirrus)
> /* ------------------------------------------------------------------ */
> /* cirrus (simple) display pipe */
>
> -static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_crtc *crtc,
> +static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
> const struct drm_display_mode *mode)
> {
> if (cirrus_check_size(mode->hdisplay, mode->vdisplay, NULL) < 0)
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> index 046055719245..15fb516ae2d8 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -43,7 +43,7 @@ drm_simple_kms_crtc_mode_valid(struct drm_crtc *crtc,
> /* Anything goes */
> return MODE_OK;
>
> - return pipe->funcs->mode_valid(crtc, mode);
> + return pipe->funcs->mode_valid(pipe, mode);
> }
>
> static int drm_simple_kms_crtc_check(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> index 024771a4083e..703ddc803c55 100644
> --- a/drivers/gpu/drm/pl111/pl111_display.c
> +++ b/drivers/gpu/drm/pl111/pl111_display.c
> @@ -48,10 +48,10 @@ irqreturn_t pl111_irq(int irq, void *data)
> }
>
> static enum drm_mode_status
> -pl111_mode_valid(struct drm_crtc *crtc,
> +pl111_mode_valid(struct drm_simple_display_pipe *pipe,
> const struct drm_display_mode *mode)
> {
> - struct drm_device *drm = crtc->dev;
> + struct drm_device *drm = pipe->crtc.dev;
> struct pl111_drm_dev_private *priv = drm->dev_private;
> u32 cpp = priv->variant->fb_bpp / 8;
> u64 bw;
> diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
> index 4d89cd0a60db..15afee9cf049 100644
> --- a/include/drm/drm_simple_kms_helper.h
> +++ b/include/drm/drm_simple_kms_helper.h
> @@ -49,7 +49,7 @@ struct drm_simple_display_pipe_funcs {
> *
> * drm_mode_status Enum
> */
> - enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc,
> + enum drm_mode_status (*mode_valid)(struct drm_simple_display_pipe *pipe,
> const struct drm_display_mode *mode);
>
> /**
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/simple-kms: Standardize arguments for callbacks
2019-10-22 15:55 [PATCH] drm/simple-kms: Standardize arguments for callbacks Daniel Vetter
2019-10-22 17:16 ` Thomas Zimmermann
@ 2019-10-22 17:16 ` Thomas Zimmermann
2019-10-22 19:03 ` Daniel Vetter
2019-10-23 0:43 ` kbuild test robot
` (3 subsequent siblings)
5 siblings, 1 reply; 22+ messages in thread
From: Thomas Zimmermann @ 2019-10-22 17:16 UTC (permalink / raw)
To: Daniel Vetter, DRI Development
Cc: virtualization, Gerd Hoffmann, Daniel Vetter, Emil Velikov
[-- Attachment #1.1.1: Type: text/plain, Size: 4432 bytes --]
Hi,
there are two types of callbacks in struct
drm_simple_display_pipe_funcs: functions that are genuine to simple KMS,
and functions that are merely forwarded from another structure (crtc,
plane, etc).
In the former category are enable(), disable(), check(), and update().
Those should probably receive a simple display pipe as their first argument.
In the latter category are mode_valid(), prepare_fb(), cleanup_fb(),
enable_vblank(), and disable_vblank(). IMHO those functions should
receive a pointer to the original structure as their first argument.
This type provides the context in which the operations make sense. (Even
their documentation already refers to the original structure.)
I admit that not all are as shareable as prepare_fb() and enable_fb().
But what else than boiler-plate wrappers do we get from simple display
pipe here?
Best regards
Thomas
Am 22.10.19 um 17:55 schrieb Daniel Vetter:
> Passing the wrong type feels icky, everywhere else we use the pipe as
> the first parameter. Spotted while discussing patches with Thomas
> Zimmermann.
>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Noralf Trønnes <noralf@tronnes.org>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Emil Velikov <emil.velikov@collabora.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> drivers/gpu/drm/cirrus/cirrus.c | 2 +-
> drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
> drivers/gpu/drm/pl111/pl111_display.c | 4 ++--
> include/drm/drm_simple_kms_helper.h | 2 +-
> 4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
> index 7d08d067e1a4..248c9f765c45 100644
> --- a/drivers/gpu/drm/cirrus/cirrus.c
> +++ b/drivers/gpu/drm/cirrus/cirrus.c
> @@ -390,7 +390,7 @@ static int cirrus_conn_init(struct cirrus_device *cirrus)
> /* ------------------------------------------------------------------ */
> /* cirrus (simple) display pipe */
>
> -static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_crtc *crtc,
> +static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
> const struct drm_display_mode *mode)
> {
> if (cirrus_check_size(mode->hdisplay, mode->vdisplay, NULL) < 0)
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> index 046055719245..15fb516ae2d8 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -43,7 +43,7 @@ drm_simple_kms_crtc_mode_valid(struct drm_crtc *crtc,
> /* Anything goes */
> return MODE_OK;
>
> - return pipe->funcs->mode_valid(crtc, mode);
> + return pipe->funcs->mode_valid(pipe, mode);
> }
>
> static int drm_simple_kms_crtc_check(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> index 024771a4083e..703ddc803c55 100644
> --- a/drivers/gpu/drm/pl111/pl111_display.c
> +++ b/drivers/gpu/drm/pl111/pl111_display.c
> @@ -48,10 +48,10 @@ irqreturn_t pl111_irq(int irq, void *data)
> }
>
> static enum drm_mode_status
> -pl111_mode_valid(struct drm_crtc *crtc,
> +pl111_mode_valid(struct drm_simple_display_pipe *pipe,
> const struct drm_display_mode *mode)
> {
> - struct drm_device *drm = crtc->dev;
> + struct drm_device *drm = pipe->crtc.dev;
> struct pl111_drm_dev_private *priv = drm->dev_private;
> u32 cpp = priv->variant->fb_bpp / 8;
> u64 bw;
> diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
> index 4d89cd0a60db..15afee9cf049 100644
> --- a/include/drm/drm_simple_kms_helper.h
> +++ b/include/drm/drm_simple_kms_helper.h
> @@ -49,7 +49,7 @@ struct drm_simple_display_pipe_funcs {
> *
> * drm_mode_status Enum
> */
> - enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc,
> + enum drm_mode_status (*mode_valid)(struct drm_simple_display_pipe *pipe,
> const struct drm_display_mode *mode);
>
> /**
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/simple-kms: Standardize arguments for callbacks
2019-10-22 17:16 ` Thomas Zimmermann
@ 2019-10-22 19:03 ` Daniel Vetter
2019-10-23 6:47 ` Thomas Zimmermann
2019-10-23 6:47 ` Thomas Zimmermann
0 siblings, 2 replies; 22+ messages in thread
From: Daniel Vetter @ 2019-10-22 19:03 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: Linus Walleij, DRI Development, open list:VIRTIO CORE, NET...,
Eric Anholt, Noralf Trønnes, Daniel Vetter, Emil Velikov
On Tue, Oct 22, 2019 at 7:16 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi,
>
> there are two types of callbacks in struct
> drm_simple_display_pipe_funcs: functions that are genuine to simple KMS,
> and functions that are merely forwarded from another structure (crtc,
> plane, etc).
>
> In the former category are enable(), disable(), check(), and update().
> Those should probably receive a simple display pipe as their first argument.
mode_valid _very_ much belongs to this category too, since there's
mode_valid hooks also on other objects. But simple pipe helper
condenses that down to one mode_valid hook (we could also put the
mode_valid onto encoder, wouldn't change anything).
> In the latter category are mode_valid(), prepare_fb(), cleanup_fb(),
> enable_vblank(), and disable_vblank(). IMHO those functions should
> receive a pointer to the original structure as their first argument.
> This type provides the context in which the operations make sense. (Even
> their documentation already refers to the original structure.)
Now on those you can maybe make a case that they only exist in one
object. But the entire point of simple helpers was to condense the zoo
of drm types down to one. Only reason you don't also get a
drm_simple_display_pipe_state is that this one would be a bit more
work to make work correctly. If we full on leak all the underlying
objects, then you might as well set them up yourself and set up all
the hooks, it's just a few more lines of code.
Imo for simple pipe we should go more into that direction, not less.
> I admit that not all are as shareable as prepare_fb() and enable_fb().
> But what else than boiler-plate wrappers do we get from simple display
> pipe here?
Boiler plate wrappers is pretty much the entire point of simple pipe
helpers. Anytime you're interested in the things it abstracts away
(crtc, plane, encoder) you probably want your own atomic
implementation. That's why I don't think using simple pipe for fbdev
conversion is a good fit, it's not meant to be useful for all small
drivers. Only for the _really_ simple ones.
Otherwise if we readd all the bells and whistles to simple pipe
helpers, then we just end back where we started. That's also why I
personally think explicit simple wrappers would fit better, instead of
wrestling the prepare/cleanup_fb functions to match full atomic
helpers.
-Daniel
>
> Best regards
> Thomas
>
> Am 22.10.19 um 17:55 schrieb Daniel Vetter:
> > Passing the wrong type feels icky, everywhere else we use the pipe as
> > the first parameter. Spotted while discussing patches with Thomas
> > Zimmermann.
> >
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: Noralf Trønnes <noralf@tronnes.org>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: Emil Velikov <emil.velikov@collabora.com>
> > Cc: virtualization@lists.linux-foundation.org
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > drivers/gpu/drm/cirrus/cirrus.c | 2 +-
> > drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
> > drivers/gpu/drm/pl111/pl111_display.c | 4 ++--
> > include/drm/drm_simple_kms_helper.h | 2 +-
> > 4 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
> > index 7d08d067e1a4..248c9f765c45 100644
> > --- a/drivers/gpu/drm/cirrus/cirrus.c
> > +++ b/drivers/gpu/drm/cirrus/cirrus.c
> > @@ -390,7 +390,7 @@ static int cirrus_conn_init(struct cirrus_device *cirrus)
> > /* ------------------------------------------------------------------ */
> > /* cirrus (simple) display pipe */
> >
> > -static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_crtc *crtc,
> > +static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
> > const struct drm_display_mode *mode)
> > {
> > if (cirrus_check_size(mode->hdisplay, mode->vdisplay, NULL) < 0)
> > diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> > index 046055719245..15fb516ae2d8 100644
> > --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> > +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> > @@ -43,7 +43,7 @@ drm_simple_kms_crtc_mode_valid(struct drm_crtc *crtc,
> > /* Anything goes */
> > return MODE_OK;
> >
> > - return pipe->funcs->mode_valid(crtc, mode);
> > + return pipe->funcs->mode_valid(pipe, mode);
> > }
> >
> > static int drm_simple_kms_crtc_check(struct drm_crtc *crtc,
> > diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> > index 024771a4083e..703ddc803c55 100644
> > --- a/drivers/gpu/drm/pl111/pl111_display.c
> > +++ b/drivers/gpu/drm/pl111/pl111_display.c
> > @@ -48,10 +48,10 @@ irqreturn_t pl111_irq(int irq, void *data)
> > }
> >
> > static enum drm_mode_status
> > -pl111_mode_valid(struct drm_crtc *crtc,
> > +pl111_mode_valid(struct drm_simple_display_pipe *pipe,
> > const struct drm_display_mode *mode)
> > {
> > - struct drm_device *drm = crtc->dev;
> > + struct drm_device *drm = pipe->crtc.dev;
> > struct pl111_drm_dev_private *priv = drm->dev_private;
> > u32 cpp = priv->variant->fb_bpp / 8;
> > u64 bw;
> > diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
> > index 4d89cd0a60db..15afee9cf049 100644
> > --- a/include/drm/drm_simple_kms_helper.h
> > +++ b/include/drm/drm_simple_kms_helper.h
> > @@ -49,7 +49,7 @@ struct drm_simple_display_pipe_funcs {
> > *
> > * drm_mode_status Enum
> > */
> > - enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc,
> > + enum drm_mode_status (*mode_valid)(struct drm_simple_display_pipe *pipe,
> > const struct drm_display_mode *mode);
> >
> > /**
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/simple-kms: Standardize arguments for callbacks
2019-10-22 15:55 [PATCH] drm/simple-kms: Standardize arguments for callbacks Daniel Vetter
` (2 preceding siblings ...)
2019-10-23 0:43 ` kbuild test robot
@ 2019-10-23 0:43 ` kbuild test robot
2019-10-23 5:55 ` kbuild test robot
2019-10-23 5:55 ` kbuild test robot
5 siblings, 0 replies; 22+ messages in thread
From: kbuild test robot @ 2019-10-23 0:43 UTC (permalink / raw)
Cc: kbuild-all, Daniel Vetter, DRI Development, virtualization,
Thomas Zimmermann, Daniel Vetter, Emil Velikov
[-- Attachment #1: Type: text/plain, Size: 2588 bytes --]
Hi Daniel,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v5.4-rc4 next-20191022]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/drm-simple-kms-Standardize-arguments-for-callbacks/20191023-073731
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 3b7c59a1950c75f2c0152e5a9cd77675b09233d6
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-14) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/gpu//drm/xen/xen_drm_front_kms.c:289:16: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
.mode_valid = display_mode_valid,
^~~~~~~~~~~~~~~~~~
drivers/gpu//drm/xen/xen_drm_front_kms.c:289:16: note: (near initialization for 'display_funcs.mode_valid')
cc1: some warnings being treated as errors
vim +289 drivers/gpu//drm/xen/xen_drm_front_kms.c
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 287
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 288 static const struct drm_simple_display_pipe_funcs display_funcs = {
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 @289 .mode_valid = display_mode_valid,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 290 .enable = display_enable,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 291 .disable = display_disable,
dd388ee1ecbb8c Daniel Vetter 2018-04-09 292 .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 293 .update = display_update,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 294 };
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 295
:::::: The code at line 289 was first introduced by commit
:::::: c575b7eeb89f94356997abd62d6d5a0590e259b7 drm/xen-front: Add support for Xen PV display frontend
:::::: TO: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
:::::: CC: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 70172 bytes --]
[-- Attachment #3: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/simple-kms: Standardize arguments for callbacks
2019-10-22 15:55 [PATCH] drm/simple-kms: Standardize arguments for callbacks Daniel Vetter
@ 2019-10-23 0:43 ` kbuild test robot
2019-10-22 17:16 ` Thomas Zimmermann
` (4 subsequent siblings)
5 siblings, 0 replies; 22+ messages in thread
From: kbuild test robot @ 2019-10-23 0:43 UTC (permalink / raw)
Cc: kbuild-all, Daniel Vetter, DRI Development, virtualization,
Gerd Hoffmann, Thomas Zimmermann, Daniel Vetter, Emil Velikov
[-- Attachment #1: Type: text/plain, Size: 2588 bytes --]
Hi Daniel,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v5.4-rc4 next-20191022]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/drm-simple-kms-Standardize-arguments-for-callbacks/20191023-073731
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 3b7c59a1950c75f2c0152e5a9cd77675b09233d6
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-14) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/gpu//drm/xen/xen_drm_front_kms.c:289:16: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
.mode_valid = display_mode_valid,
^~~~~~~~~~~~~~~~~~
drivers/gpu//drm/xen/xen_drm_front_kms.c:289:16: note: (near initialization for 'display_funcs.mode_valid')
cc1: some warnings being treated as errors
vim +289 drivers/gpu//drm/xen/xen_drm_front_kms.c
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 287
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 288 static const struct drm_simple_display_pipe_funcs display_funcs = {
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 @289 .mode_valid = display_mode_valid,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 290 .enable = display_enable,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 291 .disable = display_disable,
dd388ee1ecbb8c Daniel Vetter 2018-04-09 292 .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 293 .update = display_update,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 294 };
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 295
:::::: The code at line 289 was first introduced by commit
:::::: c575b7eeb89f94356997abd62d6d5a0590e259b7 drm/xen-front: Add support for Xen PV display frontend
:::::: TO: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
:::::: CC: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 70172 bytes --]
[-- Attachment #3: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/simple-kms: Standardize arguments for callbacks
@ 2019-10-23 0:43 ` kbuild test robot
0 siblings, 0 replies; 22+ messages in thread
From: kbuild test robot @ 2019-10-23 0:43 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2638 bytes --]
Hi Daniel,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v5.4-rc4 next-20191022]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/drm-simple-kms-Standardize-arguments-for-callbacks/20191023-073731
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 3b7c59a1950c75f2c0152e5a9cd77675b09233d6
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-14) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/gpu//drm/xen/xen_drm_front_kms.c:289:16: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
.mode_valid = display_mode_valid,
^~~~~~~~~~~~~~~~~~
drivers/gpu//drm/xen/xen_drm_front_kms.c:289:16: note: (near initialization for 'display_funcs.mode_valid')
cc1: some warnings being treated as errors
vim +289 drivers/gpu//drm/xen/xen_drm_front_kms.c
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 287
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 288 static const struct drm_simple_display_pipe_funcs display_funcs = {
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 @289 .mode_valid = display_mode_valid,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 290 .enable = display_enable,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 291 .disable = display_disable,
dd388ee1ecbb8c Daniel Vetter 2018-04-09 292 .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 293 .update = display_update,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 294 };
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 295
:::::: The code at line 289 was first introduced by commit
:::::: c575b7eeb89f94356997abd62d6d5a0590e259b7 drm/xen-front: Add support for Xen PV display frontend
:::::: TO: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
:::::: CC: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 70172 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/simple-kms: Standardize arguments for callbacks
2019-10-22 15:55 [PATCH] drm/simple-kms: Standardize arguments for callbacks Daniel Vetter
` (4 preceding siblings ...)
2019-10-23 5:55 ` kbuild test robot
@ 2019-10-23 5:55 ` kbuild test robot
5 siblings, 0 replies; 22+ messages in thread
From: kbuild test robot @ 2019-10-23 5:55 UTC (permalink / raw)
Cc: kbuild-all, Daniel Vetter, DRI Development, virtualization,
Thomas Zimmermann, Daniel Vetter, Emil Velikov
Hi Daniel,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[cannot apply to v5.4-rc4 next-20191022]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/drm-simple-kms-Standardize-arguments-for-callbacks/20191023-073731
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 3b7c59a1950c75f2c0152e5a9cd77675b09233d6
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/xen/xen_drm_front_kms.c:289:23: sparse: sparse: incorrect type in initializer (incompatible argument 1 (different base types)) @@ expected int enum drm_mode_status ( *mode_valid )( ... ) @@ got int enum drm_mode_status ( *mode_valid )( ... ) @@
>> drivers/gpu/drm/xen/xen_drm_front_kms.c:289:23: sparse: expected int enum drm_mode_status ( *mode_valid )( ... )
>> drivers/gpu/drm/xen/xen_drm_front_kms.c:289:23: sparse: got int enum drm_mode_status ( * )( ... )
vim +289 drivers/gpu/drm/xen/xen_drm_front_kms.c
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 287
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 288 static const struct drm_simple_display_pipe_funcs display_funcs = {
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 @289 .mode_valid = display_mode_valid,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 290 .enable = display_enable,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 291 .disable = display_disable,
dd388ee1ecbb8c Daniel Vetter 2018-04-09 292 .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 293 .update = display_update,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 294 };
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 295
:::::: The code at line 289 was first introduced by commit
:::::: c575b7eeb89f94356997abd62d6d5a0590e259b7 drm/xen-front: Add support for Xen PV display frontend
:::::: TO: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
:::::: CC: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/simple-kms: Standardize arguments for callbacks
2019-10-22 15:55 [PATCH] drm/simple-kms: Standardize arguments for callbacks Daniel Vetter
@ 2019-10-23 5:55 ` kbuild test robot
2019-10-22 17:16 ` Thomas Zimmermann
` (4 subsequent siblings)
5 siblings, 0 replies; 22+ messages in thread
From: kbuild test robot @ 2019-10-23 5:55 UTC (permalink / raw)
Cc: kbuild-all, Daniel Vetter, DRI Development, virtualization,
Gerd Hoffmann, Thomas Zimmermann, Daniel Vetter, Emil Velikov
Hi Daniel,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[cannot apply to v5.4-rc4 next-20191022]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/drm-simple-kms-Standardize-arguments-for-callbacks/20191023-073731
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 3b7c59a1950c75f2c0152e5a9cd77675b09233d6
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/xen/xen_drm_front_kms.c:289:23: sparse: sparse: incorrect type in initializer (incompatible argument 1 (different base types)) @@ expected int enum drm_mode_status ( *mode_valid )( ... ) @@ got int enum drm_mode_status ( *mode_valid )( ... ) @@
>> drivers/gpu/drm/xen/xen_drm_front_kms.c:289:23: sparse: expected int enum drm_mode_status ( *mode_valid )( ... )
>> drivers/gpu/drm/xen/xen_drm_front_kms.c:289:23: sparse: got int enum drm_mode_status ( * )( ... )
vim +289 drivers/gpu/drm/xen/xen_drm_front_kms.c
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 287
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 288 static const struct drm_simple_display_pipe_funcs display_funcs = {
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 @289 .mode_valid = display_mode_valid,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 290 .enable = display_enable,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 291 .disable = display_disable,
dd388ee1ecbb8c Daniel Vetter 2018-04-09 292 .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 293 .update = display_update,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 294 };
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 295
:::::: The code at line 289 was first introduced by commit
:::::: c575b7eeb89f94356997abd62d6d5a0590e259b7 drm/xen-front: Add support for Xen PV display frontend
:::::: TO: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
:::::: CC: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/simple-kms: Standardize arguments for callbacks
@ 2019-10-23 5:55 ` kbuild test robot
0 siblings, 0 replies; 22+ messages in thread
From: kbuild test robot @ 2019-10-23 5:55 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2762 bytes --]
Hi Daniel,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[cannot apply to v5.4-rc4 next-20191022]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/drm-simple-kms-Standardize-arguments-for-callbacks/20191023-073731
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 3b7c59a1950c75f2c0152e5a9cd77675b09233d6
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/xen/xen_drm_front_kms.c:289:23: sparse: sparse: incorrect type in initializer (incompatible argument 1 (different base types)) @@ expected int enum drm_mode_status ( *mode_valid )( ... ) @@ got int enum drm_mode_status ( *mode_valid )( ... ) @@
>> drivers/gpu/drm/xen/xen_drm_front_kms.c:289:23: sparse: expected int enum drm_mode_status ( *mode_valid )( ... )
>> drivers/gpu/drm/xen/xen_drm_front_kms.c:289:23: sparse: got int enum drm_mode_status ( * )( ... )
vim +289 drivers/gpu/drm/xen/xen_drm_front_kms.c
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 287
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 288 static const struct drm_simple_display_pipe_funcs display_funcs = {
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 @289 .mode_valid = display_mode_valid,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 290 .enable = display_enable,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 291 .disable = display_disable,
dd388ee1ecbb8c Daniel Vetter 2018-04-09 292 .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 293 .update = display_update,
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 294 };
c575b7eeb89f94 Oleksandr Andrushchenko 2018-04-03 295
:::::: The code at line 289 was first introduced by commit
:::::: c575b7eeb89f94356997abd62d6d5a0590e259b7 drm/xen-front: Add support for Xen PV display frontend
:::::: TO: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
:::::: CC: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/simple-kms: Standardize arguments for callbacks
2019-10-22 19:03 ` Daniel Vetter
2019-10-23 6:47 ` Thomas Zimmermann
@ 2019-10-23 6:47 ` Thomas Zimmermann
1 sibling, 0 replies; 22+ messages in thread
From: Thomas Zimmermann @ 2019-10-23 6:47 UTC (permalink / raw)
To: Daniel Vetter
Cc: Daniel Vetter, Emil Velikov, DRI Development,
open list:VIRTIO CORE, NET...
[-- Attachment #1.1.1: Type: text/plain, Size: 7298 bytes --]
Hi
Am 22.10.19 um 21:03 schrieb Daniel Vetter:
> On Tue, Oct 22, 2019 at 7:16 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Hi,
>>
>> there are two types of callbacks in struct
>> drm_simple_display_pipe_funcs: functions that are genuine to simple KMS,
>> and functions that are merely forwarded from another structure (crtc,
>> plane, etc).
>>
>> In the former category are enable(), disable(), check(), and update().
>> Those should probably receive a simple display pipe as their first argument.
>
> mode_valid _very_ much belongs to this category too, since there's
> mode_valid hooks also on other objects. But simple pipe helper
> condenses that down to one mode_valid hook (we could also put the
> mode_valid onto encoder, wouldn't change anything).
>
>> In the latter category are mode_valid(), prepare_fb(), cleanup_fb(),
>> enable_vblank(), and disable_vblank(). IMHO those functions should
>> receive a pointer to the original structure as their first argument.
>> This type provides the context in which the operations make sense. (Even
>> their documentation already refers to the original structure.)
>
> Now on those you can maybe make a case that they only exist in one
> object. But the entire point of simple helpers was to condense the zoo
> of drm types down to one. Only reason you don't also get a
> drm_simple_display_pipe_state is that this one would be a bit more
> work to make work correctly. If we full on leak all the underlying
> objects, then you might as well set them up yourself and set up all
> the hooks, it's just a few more lines of code.
>
> Imo for simple pipe we should go more into that direction, not less.
>
>> I admit that not all are as shareable as prepare_fb() and enable_fb().
>> But what else than boiler-plate wrappers do we get from simple display
>> pipe here?
>
> Boiler plate wrappers is pretty much the entire point of simple pipe
> helpers. Anytime you're interested in the things it abstracts away
> (crtc, plane, encoder) you probably want your own atomic
> implementation.
I was speaking of boiler-plate code in drivers and other helpers (e.g.,
drm_gem_fb_simple_display_pipe_prepare_fb() )
TBH I don't think it is possible to build and use simple pipe without
exposing the underlying primitives (crtc, plane, connector). This would
require a completely separate set of atomic helpers. IMHO the current
simple pipe is a mid-layer and comes with typical mid-layer problems.
Anyway, given your rational for the current design, I'll update my
recent patches for prepare_fb() to support simple pipe.
For this patch
Acked-By: Thomas Zimmermann <tzimmermann@suse.de>
Best regards
Thomas
> conversion is a good fit, it's not meant to be useful for all small
> drivers. Only for the _really_ simple ones.
>
> Otherwise if we readd all the bells and whistles to simple pipe
> helpers, then we just end back where we started. That's also why I
> personally think explicit simple wrappers would fit better, instead of
> wrestling the prepare/cleanup_fb functions to match full atomic
> helpers.
> -Daniel
>
>>
>> Best regards
>> Thomas
>>
>> Am 22.10.19 um 17:55 schrieb Daniel Vetter:
>>> Passing the wrong type feels icky, everywhere else we use the pipe as
>>> the first parameter. Spotted while discussing patches with Thomas
>>> Zimmermann.
>>>
>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>> Cc: Noralf Trønnes <noralf@tronnes.org>
>>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>>> Cc: Eric Anholt <eric@anholt.net>
>>> Cc: Emil Velikov <emil.velikov@collabora.com>
>>> Cc: virtualization@lists.linux-foundation.org
>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>> ---
>>> drivers/gpu/drm/cirrus/cirrus.c | 2 +-
>>> drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
>>> drivers/gpu/drm/pl111/pl111_display.c | 4 ++--
>>> include/drm/drm_simple_kms_helper.h | 2 +-
>>> 4 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
>>> index 7d08d067e1a4..248c9f765c45 100644
>>> --- a/drivers/gpu/drm/cirrus/cirrus.c
>>> +++ b/drivers/gpu/drm/cirrus/cirrus.c
>>> @@ -390,7 +390,7 @@ static int cirrus_conn_init(struct cirrus_device *cirrus)
>>> /* ------------------------------------------------------------------ */
>>> /* cirrus (simple) display pipe */
>>>
>>> -static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_crtc *crtc,
>>> +static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
>>> const struct drm_display_mode *mode)
>>> {
>>> if (cirrus_check_size(mode->hdisplay, mode->vdisplay, NULL) < 0)
>>> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
>>> index 046055719245..15fb516ae2d8 100644
>>> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
>>> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
>>> @@ -43,7 +43,7 @@ drm_simple_kms_crtc_mode_valid(struct drm_crtc *crtc,
>>> /* Anything goes */
>>> return MODE_OK;
>>>
>>> - return pipe->funcs->mode_valid(crtc, mode);
>>> + return pipe->funcs->mode_valid(pipe, mode);
>>> }
>>>
>>> static int drm_simple_kms_crtc_check(struct drm_crtc *crtc,
>>> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
>>> index 024771a4083e..703ddc803c55 100644
>>> --- a/drivers/gpu/drm/pl111/pl111_display.c
>>> +++ b/drivers/gpu/drm/pl111/pl111_display.c
>>> @@ -48,10 +48,10 @@ irqreturn_t pl111_irq(int irq, void *data)
>>> }
>>>
>>> static enum drm_mode_status
>>> -pl111_mode_valid(struct drm_crtc *crtc,
>>> +pl111_mode_valid(struct drm_simple_display_pipe *pipe,
>>> const struct drm_display_mode *mode)
>>> {
>>> - struct drm_device *drm = crtc->dev;
>>> + struct drm_device *drm = pipe->crtc.dev;
>>> struct pl111_drm_dev_private *priv = drm->dev_private;
>>> u32 cpp = priv->variant->fb_bpp / 8;
>>> u64 bw;
>>> diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
>>> index 4d89cd0a60db..15afee9cf049 100644
>>> --- a/include/drm/drm_simple_kms_helper.h
>>> +++ b/include/drm/drm_simple_kms_helper.h
>>> @@ -49,7 +49,7 @@ struct drm_simple_display_pipe_funcs {
>>> *
>>> * drm_mode_status Enum
>>> */
>>> - enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc,
>>> + enum drm_mode_status (*mode_valid)(struct drm_simple_display_pipe *pipe,
>>> const struct drm_display_mode *mode);
>>>
>>> /**
>>>
>>
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Felix Imendörffer
>>
>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/simple-kms: Standardize arguments for callbacks
2019-10-22 19:03 ` Daniel Vetter
@ 2019-10-23 6:47 ` Thomas Zimmermann
2019-10-23 8:53 ` Daniel Vetter
2019-10-23 8:53 ` Daniel Vetter
2019-10-23 6:47 ` Thomas Zimmermann
1 sibling, 2 replies; 22+ messages in thread
From: Thomas Zimmermann @ 2019-10-23 6:47 UTC (permalink / raw)
To: Daniel Vetter
Cc: Daniel Vetter, Emil Velikov, Gerd Hoffmann, DRI Development,
open list:VIRTIO CORE, NET...
[-- Attachment #1.1.1: Type: text/plain, Size: 7298 bytes --]
Hi
Am 22.10.19 um 21:03 schrieb Daniel Vetter:
> On Tue, Oct 22, 2019 at 7:16 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Hi,
>>
>> there are two types of callbacks in struct
>> drm_simple_display_pipe_funcs: functions that are genuine to simple KMS,
>> and functions that are merely forwarded from another structure (crtc,
>> plane, etc).
>>
>> In the former category are enable(), disable(), check(), and update().
>> Those should probably receive a simple display pipe as their first argument.
>
> mode_valid _very_ much belongs to this category too, since there's
> mode_valid hooks also on other objects. But simple pipe helper
> condenses that down to one mode_valid hook (we could also put the
> mode_valid onto encoder, wouldn't change anything).
>
>> In the latter category are mode_valid(), prepare_fb(), cleanup_fb(),
>> enable_vblank(), and disable_vblank(). IMHO those functions should
>> receive a pointer to the original structure as their first argument.
>> This type provides the context in which the operations make sense. (Even
>> their documentation already refers to the original structure.)
>
> Now on those you can maybe make a case that they only exist in one
> object. But the entire point of simple helpers was to condense the zoo
> of drm types down to one. Only reason you don't also get a
> drm_simple_display_pipe_state is that this one would be a bit more
> work to make work correctly. If we full on leak all the underlying
> objects, then you might as well set them up yourself and set up all
> the hooks, it's just a few more lines of code.
>
> Imo for simple pipe we should go more into that direction, not less.
>
>> I admit that not all are as shareable as prepare_fb() and enable_fb().
>> But what else than boiler-plate wrappers do we get from simple display
>> pipe here?
>
> Boiler plate wrappers is pretty much the entire point of simple pipe
> helpers. Anytime you're interested in the things it abstracts away
> (crtc, plane, encoder) you probably want your own atomic
> implementation.
I was speaking of boiler-plate code in drivers and other helpers (e.g.,
drm_gem_fb_simple_display_pipe_prepare_fb() )
TBH I don't think it is possible to build and use simple pipe without
exposing the underlying primitives (crtc, plane, connector). This would
require a completely separate set of atomic helpers. IMHO the current
simple pipe is a mid-layer and comes with typical mid-layer problems.
Anyway, given your rational for the current design, I'll update my
recent patches for prepare_fb() to support simple pipe.
For this patch
Acked-By: Thomas Zimmermann <tzimmermann@suse.de>
Best regards
Thomas
> conversion is a good fit, it's not meant to be useful for all small
> drivers. Only for the _really_ simple ones.
>
> Otherwise if we readd all the bells and whistles to simple pipe
> helpers, then we just end back where we started. That's also why I
> personally think explicit simple wrappers would fit better, instead of
> wrestling the prepare/cleanup_fb functions to match full atomic
> helpers.
> -Daniel
>
>>
>> Best regards
>> Thomas
>>
>> Am 22.10.19 um 17:55 schrieb Daniel Vetter:
>>> Passing the wrong type feels icky, everywhere else we use the pipe as
>>> the first parameter. Spotted while discussing patches with Thomas
>>> Zimmermann.
>>>
>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>> Cc: Noralf Trønnes <noralf@tronnes.org>
>>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>>> Cc: Eric Anholt <eric@anholt.net>
>>> Cc: Emil Velikov <emil.velikov@collabora.com>
>>> Cc: virtualization@lists.linux-foundation.org
>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>> ---
>>> drivers/gpu/drm/cirrus/cirrus.c | 2 +-
>>> drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
>>> drivers/gpu/drm/pl111/pl111_display.c | 4 ++--
>>> include/drm/drm_simple_kms_helper.h | 2 +-
>>> 4 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
>>> index 7d08d067e1a4..248c9f765c45 100644
>>> --- a/drivers/gpu/drm/cirrus/cirrus.c
>>> +++ b/drivers/gpu/drm/cirrus/cirrus.c
>>> @@ -390,7 +390,7 @@ static int cirrus_conn_init(struct cirrus_device *cirrus)
>>> /* ------------------------------------------------------------------ */
>>> /* cirrus (simple) display pipe */
>>>
>>> -static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_crtc *crtc,
>>> +static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
>>> const struct drm_display_mode *mode)
>>> {
>>> if (cirrus_check_size(mode->hdisplay, mode->vdisplay, NULL) < 0)
>>> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
>>> index 046055719245..15fb516ae2d8 100644
>>> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
>>> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
>>> @@ -43,7 +43,7 @@ drm_simple_kms_crtc_mode_valid(struct drm_crtc *crtc,
>>> /* Anything goes */
>>> return MODE_OK;
>>>
>>> - return pipe->funcs->mode_valid(crtc, mode);
>>> + return pipe->funcs->mode_valid(pipe, mode);
>>> }
>>>
>>> static int drm_simple_kms_crtc_check(struct drm_crtc *crtc,
>>> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
>>> index 024771a4083e..703ddc803c55 100644
>>> --- a/drivers/gpu/drm/pl111/pl111_display.c
>>> +++ b/drivers/gpu/drm/pl111/pl111_display.c
>>> @@ -48,10 +48,10 @@ irqreturn_t pl111_irq(int irq, void *data)
>>> }
>>>
>>> static enum drm_mode_status
>>> -pl111_mode_valid(struct drm_crtc *crtc,
>>> +pl111_mode_valid(struct drm_simple_display_pipe *pipe,
>>> const struct drm_display_mode *mode)
>>> {
>>> - struct drm_device *drm = crtc->dev;
>>> + struct drm_device *drm = pipe->crtc.dev;
>>> struct pl111_drm_dev_private *priv = drm->dev_private;
>>> u32 cpp = priv->variant->fb_bpp / 8;
>>> u64 bw;
>>> diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
>>> index 4d89cd0a60db..15afee9cf049 100644
>>> --- a/include/drm/drm_simple_kms_helper.h
>>> +++ b/include/drm/drm_simple_kms_helper.h
>>> @@ -49,7 +49,7 @@ struct drm_simple_display_pipe_funcs {
>>> *
>>> * drm_mode_status Enum
>>> */
>>> - enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc,
>>> + enum drm_mode_status (*mode_valid)(struct drm_simple_display_pipe *pipe,
>>> const struct drm_display_mode *mode);
>>>
>>> /**
>>>
>>
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Felix Imendörffer
>>
>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/simple-kms: Standardize arguments for callbacks
2019-10-23 6:47 ` Thomas Zimmermann
2019-10-23 8:53 ` Daniel Vetter
@ 2019-10-23 8:53 ` Daniel Vetter
1 sibling, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2019-10-23 8:53 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: Daniel Vetter, DRI Development, open list:VIRTIO CORE, NET...,
Daniel Vetter, Emil Velikov
On Wed, Oct 23, 2019 at 08:47:57AM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 22.10.19 um 21:03 schrieb Daniel Vetter:
> > On Tue, Oct 22, 2019 at 7:16 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >>
> >> Hi,
> >>
> >> there are two types of callbacks in struct
> >> drm_simple_display_pipe_funcs: functions that are genuine to simple KMS,
> >> and functions that are merely forwarded from another structure (crtc,
> >> plane, etc).
> >>
> >> In the former category are enable(), disable(), check(), and update().
> >> Those should probably receive a simple display pipe as their first argument.
> >
> > mode_valid _very_ much belongs to this category too, since there's
> > mode_valid hooks also on other objects. But simple pipe helper
> > condenses that down to one mode_valid hook (we could also put the
> > mode_valid onto encoder, wouldn't change anything).
> >
> >> In the latter category are mode_valid(), prepare_fb(), cleanup_fb(),
> >> enable_vblank(), and disable_vblank(). IMHO those functions should
> >> receive a pointer to the original structure as their first argument.
> >> This type provides the context in which the operations make sense. (Even
> >> their documentation already refers to the original structure.)
> >
> > Now on those you can maybe make a case that they only exist in one
> > object. But the entire point of simple helpers was to condense the zoo
> > of drm types down to one. Only reason you don't also get a
> > drm_simple_display_pipe_state is that this one would be a bit more
> > work to make work correctly. If we full on leak all the underlying
> > objects, then you might as well set them up yourself and set up all
> > the hooks, it's just a few more lines of code.
> >
> > Imo for simple pipe we should go more into that direction, not less.
> >
> >> I admit that not all are as shareable as prepare_fb() and enable_fb().
> >> But what else than boiler-plate wrappers do we get from simple display
> >> pipe here?
> >
> > Boiler plate wrappers is pretty much the entire point of simple pipe
> > helpers. Anytime you're interested in the things it abstracts away
> > (crtc, plane, encoder) you probably want your own atomic
> > implementation.
>
> I was speaking of boiler-plate code in drivers and other helpers (e.g.,
> drm_gem_fb_simple_display_pipe_prepare_fb() )
>
> TBH I don't think it is possible to build and use simple pipe without
> exposing the underlying primitives (crtc, plane, connector). This would
> require a completely separate set of atomic helpers. IMHO the current
> simple pipe is a mid-layer and comes with typical mid-layer problems.
Helpers can be midlayers, as long as their optional. And for simple I
agree it's not a perfect illusion, it's a tradeoff between having a huge
helper library that remaps everything and still enabling useful code and
complexity savings in the tiny drivers.
Maybe another rule of thumb: If your driver has more than one source file,
simple pipe is maybe not what you're looking for. Exceptions apply ofc.
simple pipe was designed for drm/tiny, and it utterly excels at that. But
that doesn't make it a general purpose tool.
> Anyway, given your rational for the current design, I'll update my
> recent patches for prepare_fb() to support simple pipe.
>
> For this patch
>
> Acked-By: Thomas Zimmermann <tzimmermann@suse.de>
Thanks, will apply.
-Daniel
>
> Best regards
> Thomas
>
> > conversion is a good fit, it's not meant to be useful for all small
> > drivers. Only for the _really_ simple ones.
> >
> > Otherwise if we readd all the bells and whistles to simple pipe
> > helpers, then we just end back where we started. That's also why I
> > personally think explicit simple wrappers would fit better, instead of
> > wrestling the prepare/cleanup_fb functions to match full atomic
> > helpers.
> > -Daniel
> >
> >>
> >> Best regards
> >> Thomas
> >>
> >> Am 22.10.19 um 17:55 schrieb Daniel Vetter:
> >>> Passing the wrong type feels icky, everywhere else we use the pipe as
> >>> the first parameter. Spotted while discussing patches with Thomas
> >>> Zimmermann.
> >>>
> >>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> >>> Cc: Noralf Trønnes <noralf@tronnes.org>
> >>> Cc: Gerd Hoffmann <kraxel@redhat.com>
> >>> Cc: Eric Anholt <eric@anholt.net>
> >>> Cc: Emil Velikov <emil.velikov@collabora.com>
> >>> Cc: virtualization@lists.linux-foundation.org
> >>> Cc: Linus Walleij <linus.walleij@linaro.org>
> >>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> >>> ---
> >>> drivers/gpu/drm/cirrus/cirrus.c | 2 +-
> >>> drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
> >>> drivers/gpu/drm/pl111/pl111_display.c | 4 ++--
> >>> include/drm/drm_simple_kms_helper.h | 2 +-
> >>> 4 files changed, 5 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
> >>> index 7d08d067e1a4..248c9f765c45 100644
> >>> --- a/drivers/gpu/drm/cirrus/cirrus.c
> >>> +++ b/drivers/gpu/drm/cirrus/cirrus.c
> >>> @@ -390,7 +390,7 @@ static int cirrus_conn_init(struct cirrus_device *cirrus)
> >>> /* ------------------------------------------------------------------ */
> >>> /* cirrus (simple) display pipe */
> >>>
> >>> -static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_crtc *crtc,
> >>> +static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
> >>> const struct drm_display_mode *mode)
> >>> {
> >>> if (cirrus_check_size(mode->hdisplay, mode->vdisplay, NULL) < 0)
> >>> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> >>> index 046055719245..15fb516ae2d8 100644
> >>> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> >>> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> >>> @@ -43,7 +43,7 @@ drm_simple_kms_crtc_mode_valid(struct drm_crtc *crtc,
> >>> /* Anything goes */
> >>> return MODE_OK;
> >>>
> >>> - return pipe->funcs->mode_valid(crtc, mode);
> >>> + return pipe->funcs->mode_valid(pipe, mode);
> >>> }
> >>>
> >>> static int drm_simple_kms_crtc_check(struct drm_crtc *crtc,
> >>> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> >>> index 024771a4083e..703ddc803c55 100644
> >>> --- a/drivers/gpu/drm/pl111/pl111_display.c
> >>> +++ b/drivers/gpu/drm/pl111/pl111_display.c
> >>> @@ -48,10 +48,10 @@ irqreturn_t pl111_irq(int irq, void *data)
> >>> }
> >>>
> >>> static enum drm_mode_status
> >>> -pl111_mode_valid(struct drm_crtc *crtc,
> >>> +pl111_mode_valid(struct drm_simple_display_pipe *pipe,
> >>> const struct drm_display_mode *mode)
> >>> {
> >>> - struct drm_device *drm = crtc->dev;
> >>> + struct drm_device *drm = pipe->crtc.dev;
> >>> struct pl111_drm_dev_private *priv = drm->dev_private;
> >>> u32 cpp = priv->variant->fb_bpp / 8;
> >>> u64 bw;
> >>> diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
> >>> index 4d89cd0a60db..15afee9cf049 100644
> >>> --- a/include/drm/drm_simple_kms_helper.h
> >>> +++ b/include/drm/drm_simple_kms_helper.h
> >>> @@ -49,7 +49,7 @@ struct drm_simple_display_pipe_funcs {
> >>> *
> >>> * drm_mode_status Enum
> >>> */
> >>> - enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc,
> >>> + enum drm_mode_status (*mode_valid)(struct drm_simple_display_pipe *pipe,
> >>> const struct drm_display_mode *mode);
> >>>
> >>> /**
> >>>
> >>
> >> --
> >> Thomas Zimmermann
> >> Graphics Driver Developer
> >> SUSE Software Solutions Germany GmbH
> >> Maxfeldstr. 5, 90409 Nürnberg, Germany
> >> (HRB 36809, AG Nürnberg)
> >> Geschäftsführer: Felix Imendörffer
> >>
> >
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/simple-kms: Standardize arguments for callbacks
2019-10-23 6:47 ` Thomas Zimmermann
@ 2019-10-23 8:53 ` Daniel Vetter
2019-10-23 8:53 ` Daniel Vetter
1 sibling, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2019-10-23 8:53 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: Daniel Vetter, DRI Development, open list:VIRTIO CORE, NET...,
Gerd Hoffmann, Daniel Vetter, Emil Velikov
On Wed, Oct 23, 2019 at 08:47:57AM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 22.10.19 um 21:03 schrieb Daniel Vetter:
> > On Tue, Oct 22, 2019 at 7:16 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >>
> >> Hi,
> >>
> >> there are two types of callbacks in struct
> >> drm_simple_display_pipe_funcs: functions that are genuine to simple KMS,
> >> and functions that are merely forwarded from another structure (crtc,
> >> plane, etc).
> >>
> >> In the former category are enable(), disable(), check(), and update().
> >> Those should probably receive a simple display pipe as their first argument.
> >
> > mode_valid _very_ much belongs to this category too, since there's
> > mode_valid hooks also on other objects. But simple pipe helper
> > condenses that down to one mode_valid hook (we could also put the
> > mode_valid onto encoder, wouldn't change anything).
> >
> >> In the latter category are mode_valid(), prepare_fb(), cleanup_fb(),
> >> enable_vblank(), and disable_vblank(). IMHO those functions should
> >> receive a pointer to the original structure as their first argument.
> >> This type provides the context in which the operations make sense. (Even
> >> their documentation already refers to the original structure.)
> >
> > Now on those you can maybe make a case that they only exist in one
> > object. But the entire point of simple helpers was to condense the zoo
> > of drm types down to one. Only reason you don't also get a
> > drm_simple_display_pipe_state is that this one would be a bit more
> > work to make work correctly. If we full on leak all the underlying
> > objects, then you might as well set them up yourself and set up all
> > the hooks, it's just a few more lines of code.
> >
> > Imo for simple pipe we should go more into that direction, not less.
> >
> >> I admit that not all are as shareable as prepare_fb() and enable_fb().
> >> But what else than boiler-plate wrappers do we get from simple display
> >> pipe here?
> >
> > Boiler plate wrappers is pretty much the entire point of simple pipe
> > helpers. Anytime you're interested in the things it abstracts away
> > (crtc, plane, encoder) you probably want your own atomic
> > implementation.
>
> I was speaking of boiler-plate code in drivers and other helpers (e.g.,
> drm_gem_fb_simple_display_pipe_prepare_fb() )
>
> TBH I don't think it is possible to build and use simple pipe without
> exposing the underlying primitives (crtc, plane, connector). This would
> require a completely separate set of atomic helpers. IMHO the current
> simple pipe is a mid-layer and comes with typical mid-layer problems.
Helpers can be midlayers, as long as their optional. And for simple I
agree it's not a perfect illusion, it's a tradeoff between having a huge
helper library that remaps everything and still enabling useful code and
complexity savings in the tiny drivers.
Maybe another rule of thumb: If your driver has more than one source file,
simple pipe is maybe not what you're looking for. Exceptions apply ofc.
simple pipe was designed for drm/tiny, and it utterly excels at that. But
that doesn't make it a general purpose tool.
> Anyway, given your rational for the current design, I'll update my
> recent patches for prepare_fb() to support simple pipe.
>
> For this patch
>
> Acked-By: Thomas Zimmermann <tzimmermann@suse.de>
Thanks, will apply.
-Daniel
>
> Best regards
> Thomas
>
> > conversion is a good fit, it's not meant to be useful for all small
> > drivers. Only for the _really_ simple ones.
> >
> > Otherwise if we readd all the bells and whistles to simple pipe
> > helpers, then we just end back where we started. That's also why I
> > personally think explicit simple wrappers would fit better, instead of
> > wrestling the prepare/cleanup_fb functions to match full atomic
> > helpers.
> > -Daniel
> >
> >>
> >> Best regards
> >> Thomas
> >>
> >> Am 22.10.19 um 17:55 schrieb Daniel Vetter:
> >>> Passing the wrong type feels icky, everywhere else we use the pipe as
> >>> the first parameter. Spotted while discussing patches with Thomas
> >>> Zimmermann.
> >>>
> >>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> >>> Cc: Noralf Trønnes <noralf@tronnes.org>
> >>> Cc: Gerd Hoffmann <kraxel@redhat.com>
> >>> Cc: Eric Anholt <eric@anholt.net>
> >>> Cc: Emil Velikov <emil.velikov@collabora.com>
> >>> Cc: virtualization@lists.linux-foundation.org
> >>> Cc: Linus Walleij <linus.walleij@linaro.org>
> >>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> >>> ---
> >>> drivers/gpu/drm/cirrus/cirrus.c | 2 +-
> >>> drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
> >>> drivers/gpu/drm/pl111/pl111_display.c | 4 ++--
> >>> include/drm/drm_simple_kms_helper.h | 2 +-
> >>> 4 files changed, 5 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
> >>> index 7d08d067e1a4..248c9f765c45 100644
> >>> --- a/drivers/gpu/drm/cirrus/cirrus.c
> >>> +++ b/drivers/gpu/drm/cirrus/cirrus.c
> >>> @@ -390,7 +390,7 @@ static int cirrus_conn_init(struct cirrus_device *cirrus)
> >>> /* ------------------------------------------------------------------ */
> >>> /* cirrus (simple) display pipe */
> >>>
> >>> -static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_crtc *crtc,
> >>> +static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
> >>> const struct drm_display_mode *mode)
> >>> {
> >>> if (cirrus_check_size(mode->hdisplay, mode->vdisplay, NULL) < 0)
> >>> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> >>> index 046055719245..15fb516ae2d8 100644
> >>> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> >>> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> >>> @@ -43,7 +43,7 @@ drm_simple_kms_crtc_mode_valid(struct drm_crtc *crtc,
> >>> /* Anything goes */
> >>> return MODE_OK;
> >>>
> >>> - return pipe->funcs->mode_valid(crtc, mode);
> >>> + return pipe->funcs->mode_valid(pipe, mode);
> >>> }
> >>>
> >>> static int drm_simple_kms_crtc_check(struct drm_crtc *crtc,
> >>> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> >>> index 024771a4083e..703ddc803c55 100644
> >>> --- a/drivers/gpu/drm/pl111/pl111_display.c
> >>> +++ b/drivers/gpu/drm/pl111/pl111_display.c
> >>> @@ -48,10 +48,10 @@ irqreturn_t pl111_irq(int irq, void *data)
> >>> }
> >>>
> >>> static enum drm_mode_status
> >>> -pl111_mode_valid(struct drm_crtc *crtc,
> >>> +pl111_mode_valid(struct drm_simple_display_pipe *pipe,
> >>> const struct drm_display_mode *mode)
> >>> {
> >>> - struct drm_device *drm = crtc->dev;
> >>> + struct drm_device *drm = pipe->crtc.dev;
> >>> struct pl111_drm_dev_private *priv = drm->dev_private;
> >>> u32 cpp = priv->variant->fb_bpp / 8;
> >>> u64 bw;
> >>> diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
> >>> index 4d89cd0a60db..15afee9cf049 100644
> >>> --- a/include/drm/drm_simple_kms_helper.h
> >>> +++ b/include/drm/drm_simple_kms_helper.h
> >>> @@ -49,7 +49,7 @@ struct drm_simple_display_pipe_funcs {
> >>> *
> >>> * drm_mode_status Enum
> >>> */
> >>> - enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc,
> >>> + enum drm_mode_status (*mode_valid)(struct drm_simple_display_pipe *pipe,
> >>> const struct drm_display_mode *mode);
> >>>
> >>> /**
> >>>
> >>
> >> --
> >> Thomas Zimmermann
> >> Graphics Driver Developer
> >> SUSE Software Solutions Germany GmbH
> >> Maxfeldstr. 5, 90409 Nürnberg, Germany
> >> (HRB 36809, AG Nürnberg)
> >> Geschäftsführer: Felix Imendörffer
> >>
> >
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
>
--
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] 22+ messages in thread
* Re: [PATCH] drm/simple-kms: Standardize arguments for callbacks
2019-10-23 15:40 ` Linus Walleij
(?)
(?)
@ 2019-10-24 11:55 ` Daniel Vetter
-1 siblings, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2019-10-24 11:55 UTC (permalink / raw)
To: Linus Walleij
Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
virtualization, Eric Anholt, Noralf Trønnes,
Thomas Zimmermann, Daniel Vetter, Emil Velikov
On Wed, Oct 23, 2019 at 05:40:32PM +0200, Linus Walleij wrote:
> On Wed, Oct 23, 2019 at 12:13 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> > Passing the wrong type feels icky, everywhere else we use the pipe as
> > the first parameter. Spotted while discussing patches with Thomas
> > Zimmermann.
> >
> > v2: Make xen compile correctly
> >
> > Acked-By: Thomas Zimmermann <tzimmermann@suse.de> (v1)
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: Noralf Trønnes <noralf@tronnes.org>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: Emil Velikov <emil.velikov@collabora.com>
> > Cc: virtualization@lists.linux-foundation.org
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>
> Makes perfect sense.
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Thanks for taking a look, applied.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/simple-kms: Standardize arguments for callbacks
@ 2019-10-24 11:55 ` Daniel Vetter
0 siblings, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2019-10-24 11:55 UTC (permalink / raw)
To: Linus Walleij
Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
virtualization, Noralf Trønnes, Gerd Hoffmann,
Thomas Zimmermann, Daniel Vetter, Emil Velikov
On Wed, Oct 23, 2019 at 05:40:32PM +0200, Linus Walleij wrote:
> On Wed, Oct 23, 2019 at 12:13 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> > Passing the wrong type feels icky, everywhere else we use the pipe as
> > the first parameter. Spotted while discussing patches with Thomas
> > Zimmermann.
> >
> > v2: Make xen compile correctly
> >
> > Acked-By: Thomas Zimmermann <tzimmermann@suse.de> (v1)
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: Noralf Trønnes <noralf@tronnes.org>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: Emil Velikov <emil.velikov@collabora.com>
> > Cc: virtualization@lists.linux-foundation.org
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>
> Makes perfect sense.
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Thanks for taking a look, applied.
-Daniel
--
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] 22+ messages in thread
* Re: [PATCH] drm/simple-kms: Standardize arguments for callbacks
@ 2019-10-24 11:55 ` Daniel Vetter
0 siblings, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2019-10-24 11:55 UTC (permalink / raw)
To: Linus Walleij
Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
virtualization, Gerd Hoffmann, Thomas Zimmermann, Daniel Vetter,
Emil Velikov
On Wed, Oct 23, 2019 at 05:40:32PM +0200, Linus Walleij wrote:
> On Wed, Oct 23, 2019 at 12:13 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> > Passing the wrong type feels icky, everywhere else we use the pipe as
> > the first parameter. Spotted while discussing patches with Thomas
> > Zimmermann.
> >
> > v2: Make xen compile correctly
> >
> > Acked-By: Thomas Zimmermann <tzimmermann@suse.de> (v1)
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: Noralf Trønnes <noralf@tronnes.org>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: Emil Velikov <emil.velikov@collabora.com>
> > Cc: virtualization@lists.linux-foundation.org
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>
> Makes perfect sense.
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Thanks for taking a look, applied.
-Daniel
--
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] 22+ messages in thread
* Re: [PATCH] drm/simple-kms: Standardize arguments for callbacks
@ 2019-10-23 15:40 ` Linus Walleij
0 siblings, 0 replies; 22+ messages in thread
From: Linus Walleij @ 2019-10-23 15:40 UTC (permalink / raw)
To: Daniel Vetter
Cc: Intel Graphics Development, DRI Development, virtualization,
Eric Anholt, Noralf Trønnes, Thomas Zimmermann,
Daniel Vetter, Emil Velikov
On Wed, Oct 23, 2019 at 12:13 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Passing the wrong type feels icky, everywhere else we use the pipe as
> the first parameter. Spotted while discussing patches with Thomas
> Zimmermann.
>
> v2: Make xen compile correctly
>
> Acked-By: Thomas Zimmermann <tzimmermann@suse.de> (v1)
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Noralf Trønnes <noralf@tronnes.org>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Emil Velikov <emil.velikov@collabora.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Makes perfect sense.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/simple-kms: Standardize arguments for callbacks
@ 2019-10-23 15:40 ` Linus Walleij
0 siblings, 0 replies; 22+ messages in thread
From: Linus Walleij @ 2019-10-23 15:40 UTC (permalink / raw)
To: Daniel Vetter
Cc: Intel Graphics Development, DRI Development, virtualization,
Gerd Hoffmann, Thomas Zimmermann, Daniel Vetter, Emil Velikov
On Wed, Oct 23, 2019 at 12:13 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Passing the wrong type feels icky, everywhere else we use the pipe as
> the first parameter. Spotted while discussing patches with Thomas
> Zimmermann.
>
> v2: Make xen compile correctly
>
> Acked-By: Thomas Zimmermann <tzimmermann@suse.de> (v1)
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Noralf Trønnes <noralf@tronnes.org>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Emil Velikov <emil.velikov@collabora.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Makes perfect sense.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] drm/simple-kms: Standardize arguments for callbacks
@ 2019-10-23 10:12 ` Daniel Vetter
0 siblings, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2019-10-23 10:12 UTC (permalink / raw)
To: DRI Development
Cc: Daniel Vetter, Intel Graphics Development, virtualization,
Eric Anholt, Noralf Trønnes, Thomas Zimmermann,
Daniel Vetter, Linus Walleij, Emil Velikov
Passing the wrong type feels icky, everywhere else we use the pipe as
the first parameter. Spotted while discussing patches with Thomas
Zimmermann.
v2: Make xen compile correctly
Acked-By: Thomas Zimmermann <tzimmermann@suse.de> (v1)
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Noralf Trønnes <noralf@tronnes.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Emil Velikov <emil.velikov@collabora.com>
Cc: virtualization@lists.linux-foundation.org
Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/cirrus/cirrus.c | 2 +-
drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
drivers/gpu/drm/pl111/pl111_display.c | 4 ++--
drivers/gpu/drm/xen/xen_drm_front_kms.c | 7 ++++---
include/drm/drm_simple_kms_helper.h | 2 +-
5 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
index 7d08d067e1a4..248c9f765c45 100644
--- a/drivers/gpu/drm/cirrus/cirrus.c
+++ b/drivers/gpu/drm/cirrus/cirrus.c
@@ -390,7 +390,7 @@ static int cirrus_conn_init(struct cirrus_device *cirrus)
/* ------------------------------------------------------------------ */
/* cirrus (simple) display pipe */
-static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_crtc *crtc,
+static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
const struct drm_display_mode *mode)
{
if (cirrus_check_size(mode->hdisplay, mode->vdisplay, NULL) < 0)
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index 046055719245..15fb516ae2d8 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -43,7 +43,7 @@ drm_simple_kms_crtc_mode_valid(struct drm_crtc *crtc,
/* Anything goes */
return MODE_OK;
- return pipe->funcs->mode_valid(crtc, mode);
+ return pipe->funcs->mode_valid(pipe, mode);
}
static int drm_simple_kms_crtc_check(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 024771a4083e..703ddc803c55 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -48,10 +48,10 @@ irqreturn_t pl111_irq(int irq, void *data)
}
static enum drm_mode_status
-pl111_mode_valid(struct drm_crtc *crtc,
+pl111_mode_valid(struct drm_simple_display_pipe *pipe,
const struct drm_display_mode *mode)
{
- struct drm_device *drm = crtc->dev;
+ struct drm_device *drm = pipe->crtc.dev;
struct pl111_drm_dev_private *priv = drm->dev_private;
u32 cpp = priv->variant->fb_bpp / 8;
u64 bw;
diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.c b/drivers/gpu/drm/xen/xen_drm_front_kms.c
index 21ad1c359b61..ff506bc99414 100644
--- a/drivers/gpu/drm/xen/xen_drm_front_kms.c
+++ b/drivers/gpu/drm/xen/xen_drm_front_kms.c
@@ -270,11 +270,12 @@ static void display_update(struct drm_simple_display_pipe *pipe,
}
static enum drm_mode_status
-display_mode_valid(struct drm_crtc *crtc, const struct drm_display_mode *mode)
+display_mode_valid(struct drm_simple_display_pipe *pipe,
+ const struct drm_display_mode *mode)
{
struct xen_drm_front_drm_pipeline *pipeline =
- container_of(crtc, struct xen_drm_front_drm_pipeline,
- pipe.crtc);
+ container_of(pipe, struct xen_drm_front_drm_pipeline,
+ pipe);
if (mode->hdisplay != pipeline->width)
return MODE_ERROR;
diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
index 4d89cd0a60db..15afee9cf049 100644
--- a/include/drm/drm_simple_kms_helper.h
+++ b/include/drm/drm_simple_kms_helper.h
@@ -49,7 +49,7 @@ struct drm_simple_display_pipe_funcs {
*
* drm_mode_status Enum
*/
- enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc,
+ enum drm_mode_status (*mode_valid)(struct drm_simple_display_pipe *pipe,
const struct drm_display_mode *mode);
/**
--
2.23.0
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH] drm/simple-kms: Standardize arguments for callbacks
@ 2019-10-23 10:12 ` Daniel Vetter
0 siblings, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2019-10-23 10:12 UTC (permalink / raw)
To: DRI Development
Cc: Daniel Vetter, Intel Graphics Development, virtualization,
Gerd Hoffmann, Thomas Zimmermann, Daniel Vetter, Emil Velikov
Passing the wrong type feels icky, everywhere else we use the pipe as
the first parameter. Spotted while discussing patches with Thomas
Zimmermann.
v2: Make xen compile correctly
Acked-By: Thomas Zimmermann <tzimmermann@suse.de> (v1)
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Noralf Trønnes <noralf@tronnes.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Emil Velikov <emil.velikov@collabora.com>
Cc: virtualization@lists.linux-foundation.org
Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/cirrus/cirrus.c | 2 +-
drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
drivers/gpu/drm/pl111/pl111_display.c | 4 ++--
drivers/gpu/drm/xen/xen_drm_front_kms.c | 7 ++++---
include/drm/drm_simple_kms_helper.h | 2 +-
5 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
index 7d08d067e1a4..248c9f765c45 100644
--- a/drivers/gpu/drm/cirrus/cirrus.c
+++ b/drivers/gpu/drm/cirrus/cirrus.c
@@ -390,7 +390,7 @@ static int cirrus_conn_init(struct cirrus_device *cirrus)
/* ------------------------------------------------------------------ */
/* cirrus (simple) display pipe */
-static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_crtc *crtc,
+static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
const struct drm_display_mode *mode)
{
if (cirrus_check_size(mode->hdisplay, mode->vdisplay, NULL) < 0)
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index 046055719245..15fb516ae2d8 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -43,7 +43,7 @@ drm_simple_kms_crtc_mode_valid(struct drm_crtc *crtc,
/* Anything goes */
return MODE_OK;
- return pipe->funcs->mode_valid(crtc, mode);
+ return pipe->funcs->mode_valid(pipe, mode);
}
static int drm_simple_kms_crtc_check(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 024771a4083e..703ddc803c55 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -48,10 +48,10 @@ irqreturn_t pl111_irq(int irq, void *data)
}
static enum drm_mode_status
-pl111_mode_valid(struct drm_crtc *crtc,
+pl111_mode_valid(struct drm_simple_display_pipe *pipe,
const struct drm_display_mode *mode)
{
- struct drm_device *drm = crtc->dev;
+ struct drm_device *drm = pipe->crtc.dev;
struct pl111_drm_dev_private *priv = drm->dev_private;
u32 cpp = priv->variant->fb_bpp / 8;
u64 bw;
diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.c b/drivers/gpu/drm/xen/xen_drm_front_kms.c
index 21ad1c359b61..ff506bc99414 100644
--- a/drivers/gpu/drm/xen/xen_drm_front_kms.c
+++ b/drivers/gpu/drm/xen/xen_drm_front_kms.c
@@ -270,11 +270,12 @@ static void display_update(struct drm_simple_display_pipe *pipe,
}
static enum drm_mode_status
-display_mode_valid(struct drm_crtc *crtc, const struct drm_display_mode *mode)
+display_mode_valid(struct drm_simple_display_pipe *pipe,
+ const struct drm_display_mode *mode)
{
struct xen_drm_front_drm_pipeline *pipeline =
- container_of(crtc, struct xen_drm_front_drm_pipeline,
- pipe.crtc);
+ container_of(pipe, struct xen_drm_front_drm_pipeline,
+ pipe);
if (mode->hdisplay != pipeline->width)
return MODE_ERROR;
diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
index 4d89cd0a60db..15afee9cf049 100644
--- a/include/drm/drm_simple_kms_helper.h
+++ b/include/drm/drm_simple_kms_helper.h
@@ -49,7 +49,7 @@ struct drm_simple_display_pipe_funcs {
*
* drm_mode_status Enum
*/
- enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc,
+ enum drm_mode_status (*mode_valid)(struct drm_simple_display_pipe *pipe,
const struct drm_display_mode *mode);
/**
--
2.23.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH] drm/simple-kms: Standardize arguments for callbacks
@ 2019-10-22 15:55 Daniel Vetter
0 siblings, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2019-10-22 15:55 UTC (permalink / raw)
To: DRI Development
Cc: Daniel Vetter, Linus Walleij, virtualization, Eric Anholt,
Noralf Trønnes, Thomas Zimmermann, Daniel Vetter,
Emil Velikov
Passing the wrong type feels icky, everywhere else we use the pipe as
the first parameter. Spotted while discussing patches with Thomas
Zimmermann.
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Noralf Trønnes <noralf@tronnes.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Emil Velikov <emil.velikov@collabora.com>
Cc: virtualization@lists.linux-foundation.org
Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/cirrus/cirrus.c | 2 +-
drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
drivers/gpu/drm/pl111/pl111_display.c | 4 ++--
include/drm/drm_simple_kms_helper.h | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
index 7d08d067e1a4..248c9f765c45 100644
--- a/drivers/gpu/drm/cirrus/cirrus.c
+++ b/drivers/gpu/drm/cirrus/cirrus.c
@@ -390,7 +390,7 @@ static int cirrus_conn_init(struct cirrus_device *cirrus)
/* ------------------------------------------------------------------ */
/* cirrus (simple) display pipe */
-static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_crtc *crtc,
+static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
const struct drm_display_mode *mode)
{
if (cirrus_check_size(mode->hdisplay, mode->vdisplay, NULL) < 0)
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index 046055719245..15fb516ae2d8 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -43,7 +43,7 @@ drm_simple_kms_crtc_mode_valid(struct drm_crtc *crtc,
/* Anything goes */
return MODE_OK;
- return pipe->funcs->mode_valid(crtc, mode);
+ return pipe->funcs->mode_valid(pipe, mode);
}
static int drm_simple_kms_crtc_check(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 024771a4083e..703ddc803c55 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -48,10 +48,10 @@ irqreturn_t pl111_irq(int irq, void *data)
}
static enum drm_mode_status
-pl111_mode_valid(struct drm_crtc *crtc,
+pl111_mode_valid(struct drm_simple_display_pipe *pipe,
const struct drm_display_mode *mode)
{
- struct drm_device *drm = crtc->dev;
+ struct drm_device *drm = pipe->crtc.dev;
struct pl111_drm_dev_private *priv = drm->dev_private;
u32 cpp = priv->variant->fb_bpp / 8;
u64 bw;
diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
index 4d89cd0a60db..15afee9cf049 100644
--- a/include/drm/drm_simple_kms_helper.h
+++ b/include/drm/drm_simple_kms_helper.h
@@ -49,7 +49,7 @@ struct drm_simple_display_pipe_funcs {
*
* drm_mode_status Enum
*/
- enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc,
+ enum drm_mode_status (*mode_valid)(struct drm_simple_display_pipe *pipe,
const struct drm_display_mode *mode);
/**
--
2.23.0
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 22+ messages in thread
end of thread, other threads:[~2019-10-24 11:55 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-22 15:55 [PATCH] drm/simple-kms: Standardize arguments for callbacks Daniel Vetter
2019-10-22 17:16 ` Thomas Zimmermann
2019-10-22 17:16 ` Thomas Zimmermann
2019-10-22 19:03 ` Daniel Vetter
2019-10-23 6:47 ` Thomas Zimmermann
2019-10-23 8:53 ` Daniel Vetter
2019-10-23 8:53 ` Daniel Vetter
2019-10-23 6:47 ` Thomas Zimmermann
2019-10-23 0:43 ` kbuild test robot
2019-10-23 0:43 ` kbuild test robot
2019-10-23 0:43 ` kbuild test robot
2019-10-23 5:55 ` kbuild test robot
2019-10-23 5:55 ` kbuild test robot
2019-10-23 5:55 ` kbuild test robot
2019-10-22 15:55 Daniel Vetter
2019-10-23 10:12 Daniel Vetter
2019-10-23 10:12 ` Daniel Vetter
2019-10-23 15:40 ` Linus Walleij
2019-10-23 15:40 ` Linus Walleij
2019-10-24 11:55 ` Daniel Vetter
2019-10-24 11:55 ` Daniel Vetter
2019-10-24 11:55 ` Daniel Vetter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.