From: Daniel Vetter <daniel@ffwll.ch> To: Thomas Zimmermann <tzimmermann@suse.de> Cc: airlied@linux.ie, dri-devel@lists.freedesktop.org, maarten.lankhorst@linux.intel.com, mripard@kernel.org, virtualization@lists.linux-foundation.org, noralf@tronnes.org, daniel@ffwll.ch, alexander.deucher@amd.com, spice-devel@lists.freedesktop.org, sam@ravnborg.org, emil.velikov@collabora.com Subject: Re: [PATCH 2/6] drm: Add drm_simple_encoder_{init,create}() Date: Fri, 7 Feb 2020 14:37:20 +0100 [thread overview] Message-ID: <20200207133720.GZ43062@phenom.ffwll.local> (raw) In-Reply-To: <20200207084135.4524-3-tzimmermann@suse.de> On Fri, Feb 07, 2020 at 09:41:31AM +0100, Thomas Zimmermann wrote: > The simple-encoder helpers initialize an encoder with an empty > implementation. This covers the requirements of most of the existing > DRM drivers. A call to drm_simple_encoder_create() allocates and > initializes an encoder instance, a call to drm_simple_encoder_init() > initializes a pre-allocated instance. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> This has quick a bit midlayer taste to it ... I think having this as a helper would be cleaner ... The other bit is drm_encoder->possible_crtcs. If we do create a helper for these, lets at least try to make them not suck too badly :-) Otherwise I guess it would be time to officially document what exactly possible_crtcs == 0 means from an uabi pov. -Daniel > --- > drivers/gpu/drm/drm_encoder.c | 116 ++++++++++++++++++++++++++++++++++ > include/drm/drm_encoder.h | 10 +++ > 2 files changed, 126 insertions(+) > > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c > index ffe691a1bf34..1a65cab1f310 100644 > --- a/drivers/gpu/drm/drm_encoder.c > +++ b/drivers/gpu/drm/drm_encoder.c > @@ -178,6 +178,122 @@ int drm_encoder_init(struct drm_device *dev, > } > EXPORT_SYMBOL(drm_encoder_init); > > +static const struct drm_encoder_funcs drm_simple_encoder_funcs_cleanup = { > + .destroy = drm_encoder_cleanup, > +}; > + > +/** > + * drm_simple_encoder_init - Init a preallocated encoder > + * @dev: drm device > + * @funcs: callbacks for this encoder > + * @encoder_type: user visible type of the encoder > + * @name: printf style format string for the encoder name, or NULL > + * for default name > + * > + * Initialises a preallocated encoder that has no further functionality. The > + * encoder will be released automatically. > + * > + * Returns: > + * Zero on success, error code on failure. > + */ > +int drm_simple_encoder_init(struct drm_device *dev, > + struct drm_encoder *encoder, > + int encoder_type, const char *name, ...) > +{ > + char *namestr = NULL; > + int ret; > + > + if (name) { > + va_list ap; > + > + va_start(ap, name); > + namestr = kvasprintf(GFP_KERNEL, name, ap); > + va_end(ap); > + if (!namestr) > + return -ENOMEM; > + } > + > + ret = __drm_encoder_init(dev, encoder, > + &drm_simple_encoder_funcs_cleanup, > + encoder_type, namestr); > + if (ret) > + goto err_kfree; > + > + return 0; > + > +err_kfree: > + if (name) > + kfree(namestr); > + return ret; > +} > +EXPORT_SYMBOL(drm_simple_encoder_init); > + > +static void drm_encoder_destroy(struct drm_encoder *encoder) > +{ > + struct drm_device *dev = encoder->dev; > + > + drm_encoder_cleanup(encoder); > + devm_kfree(dev->dev, encoder); > +} > + > +static const struct drm_encoder_funcs drm_simple_encoder_funcs_destroy = { > + .destroy = drm_encoder_destroy, > +}; > + > +/** > + * drm_simple_encoder_create - Allocate and initialize an encoder > + * @dev: drm device > + * @encoder_type: user visible type of the encoder > + * @name: printf style format string for the encoder name, or NULL for > + * default name > + * > + * Allocates and initialises an encoder that has no further functionality. The > + * encoder will be released automatically. > + * > + * Returns: > + * The encoder on success, a pointer-encoder error code on failure. > + */ > +struct drm_encoder *drm_simple_encoder_create(struct drm_device *dev, > + int encoder_type, > + const char *name, ...) > +{ > + char *namestr = NULL; > + struct drm_encoder *encoder; > + int ret; > + > + encoder = devm_kzalloc(dev->dev, sizeof(*encoder), GFP_KERNEL); > + if (!encoder) > + return ERR_PTR(-ENOMEM); > + > + if (name) { > + va_list ap; > + > + va_start(ap, name); > + namestr = kvasprintf(GFP_KERNEL, name, ap); > + va_end(ap); > + if (!namestr) { > + ret = -ENOMEM; > + goto err_devm_kfree; > + } > + } > + > + ret = __drm_encoder_init(dev, encoder, > + &drm_simple_encoder_funcs_destroy, > + encoder_type, namestr); > + if (ret) > + goto err_kfree; > + > + return encoder; > + > +err_kfree: > + if (name) > + kfree(namestr); > +err_devm_kfree: > + devm_kfree(dev->dev, encoder); > + return ERR_PTR(ret); > +} > +EXPORT_SYMBOL(drm_simple_encoder_create); > + > /** > * drm_encoder_cleanup - cleans up an initialised encoder > * @encoder: encoder to cleanup > diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h > index 5623994b6e9e..0214f6cf9de6 100644 > --- a/include/drm/drm_encoder.h > +++ b/include/drm/drm_encoder.h > @@ -190,6 +190,16 @@ int drm_encoder_init(struct drm_device *dev, > const struct drm_encoder_funcs *funcs, > int encoder_type, const char *name, ...); > > +__printf(4, 5) > +int drm_simple_encoder_init(struct drm_device *dev, > + struct drm_encoder *encoder, > + int encoder_type, const char *name, ...); > + > +__printf(3, 4) > +struct drm_encoder *drm_simple_encoder_create(struct drm_device *dev, > + int encoder_type, > + const char *name, ...); > + > /** > * drm_encoder_index - find the index of a registered encoder > * @encoder: encoder to find index for > -- > 2.25.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel@ffwll.ch> To: Thomas Zimmermann <tzimmermann@suse.de> Cc: airlied@linux.ie, dri-devel@lists.freedesktop.org, virtualization@lists.linux-foundation.org, kraxel@redhat.com, alexander.deucher@amd.com, spice-devel@lists.freedesktop.org, sam@ravnborg.org, emil.velikov@collabora.com Subject: Re: [PATCH 2/6] drm: Add drm_simple_encoder_{init,create}() Date: Fri, 7 Feb 2020 14:37:20 +0100 [thread overview] Message-ID: <20200207133720.GZ43062@phenom.ffwll.local> (raw) In-Reply-To: <20200207084135.4524-3-tzimmermann@suse.de> On Fri, Feb 07, 2020 at 09:41:31AM +0100, Thomas Zimmermann wrote: > The simple-encoder helpers initialize an encoder with an empty > implementation. This covers the requirements of most of the existing > DRM drivers. A call to drm_simple_encoder_create() allocates and > initializes an encoder instance, a call to drm_simple_encoder_init() > initializes a pre-allocated instance. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> This has quick a bit midlayer taste to it ... I think having this as a helper would be cleaner ... The other bit is drm_encoder->possible_crtcs. If we do create a helper for these, lets at least try to make them not suck too badly :-) Otherwise I guess it would be time to officially document what exactly possible_crtcs == 0 means from an uabi pov. -Daniel > --- > drivers/gpu/drm/drm_encoder.c | 116 ++++++++++++++++++++++++++++++++++ > include/drm/drm_encoder.h | 10 +++ > 2 files changed, 126 insertions(+) > > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c > index ffe691a1bf34..1a65cab1f310 100644 > --- a/drivers/gpu/drm/drm_encoder.c > +++ b/drivers/gpu/drm/drm_encoder.c > @@ -178,6 +178,122 @@ int drm_encoder_init(struct drm_device *dev, > } > EXPORT_SYMBOL(drm_encoder_init); > > +static const struct drm_encoder_funcs drm_simple_encoder_funcs_cleanup = { > + .destroy = drm_encoder_cleanup, > +}; > + > +/** > + * drm_simple_encoder_init - Init a preallocated encoder > + * @dev: drm device > + * @funcs: callbacks for this encoder > + * @encoder_type: user visible type of the encoder > + * @name: printf style format string for the encoder name, or NULL > + * for default name > + * > + * Initialises a preallocated encoder that has no further functionality. The > + * encoder will be released automatically. > + * > + * Returns: > + * Zero on success, error code on failure. > + */ > +int drm_simple_encoder_init(struct drm_device *dev, > + struct drm_encoder *encoder, > + int encoder_type, const char *name, ...) > +{ > + char *namestr = NULL; > + int ret; > + > + if (name) { > + va_list ap; > + > + va_start(ap, name); > + namestr = kvasprintf(GFP_KERNEL, name, ap); > + va_end(ap); > + if (!namestr) > + return -ENOMEM; > + } > + > + ret = __drm_encoder_init(dev, encoder, > + &drm_simple_encoder_funcs_cleanup, > + encoder_type, namestr); > + if (ret) > + goto err_kfree; > + > + return 0; > + > +err_kfree: > + if (name) > + kfree(namestr); > + return ret; > +} > +EXPORT_SYMBOL(drm_simple_encoder_init); > + > +static void drm_encoder_destroy(struct drm_encoder *encoder) > +{ > + struct drm_device *dev = encoder->dev; > + > + drm_encoder_cleanup(encoder); > + devm_kfree(dev->dev, encoder); > +} > + > +static const struct drm_encoder_funcs drm_simple_encoder_funcs_destroy = { > + .destroy = drm_encoder_destroy, > +}; > + > +/** > + * drm_simple_encoder_create - Allocate and initialize an encoder > + * @dev: drm device > + * @encoder_type: user visible type of the encoder > + * @name: printf style format string for the encoder name, or NULL for > + * default name > + * > + * Allocates and initialises an encoder that has no further functionality. The > + * encoder will be released automatically. > + * > + * Returns: > + * The encoder on success, a pointer-encoder error code on failure. > + */ > +struct drm_encoder *drm_simple_encoder_create(struct drm_device *dev, > + int encoder_type, > + const char *name, ...) > +{ > + char *namestr = NULL; > + struct drm_encoder *encoder; > + int ret; > + > + encoder = devm_kzalloc(dev->dev, sizeof(*encoder), GFP_KERNEL); > + if (!encoder) > + return ERR_PTR(-ENOMEM); > + > + if (name) { > + va_list ap; > + > + va_start(ap, name); > + namestr = kvasprintf(GFP_KERNEL, name, ap); > + va_end(ap); > + if (!namestr) { > + ret = -ENOMEM; > + goto err_devm_kfree; > + } > + } > + > + ret = __drm_encoder_init(dev, encoder, > + &drm_simple_encoder_funcs_destroy, > + encoder_type, namestr); > + if (ret) > + goto err_kfree; > + > + return encoder; > + > +err_kfree: > + if (name) > + kfree(namestr); > +err_devm_kfree: > + devm_kfree(dev->dev, encoder); > + return ERR_PTR(ret); > +} > +EXPORT_SYMBOL(drm_simple_encoder_create); > + > /** > * drm_encoder_cleanup - cleans up an initialised encoder > * @encoder: encoder to cleanup > diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h > index 5623994b6e9e..0214f6cf9de6 100644 > --- a/include/drm/drm_encoder.h > +++ b/include/drm/drm_encoder.h > @@ -190,6 +190,16 @@ int drm_encoder_init(struct drm_device *dev, > const struct drm_encoder_funcs *funcs, > int encoder_type, const char *name, ...); > > +__printf(4, 5) > +int drm_simple_encoder_init(struct drm_device *dev, > + struct drm_encoder *encoder, > + int encoder_type, const char *name, ...); > + > +__printf(3, 4) > +struct drm_encoder *drm_simple_encoder_create(struct drm_device *dev, > + int encoder_type, > + const char *name, ...); > + > /** > * drm_encoder_index - find the index of a registered encoder > * @encoder: encoder to find index for > -- > 2.25.0 > -- 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
next prev parent reply other threads:[~2020-02-07 13:37 UTC|newest] Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-02-07 8:41 [PATCH 0/6] drm: Provide a simple encoder Thomas Zimmermann 2020-02-07 8:41 ` Thomas Zimmermann 2020-02-07 8:41 ` [PATCH 1/6] drm: Move initialization of encoder into an internal function Thomas Zimmermann 2020-02-07 8:41 ` Thomas Zimmermann 2020-02-07 8:41 ` [PATCH 2/6] drm: Add drm_simple_encoder_{init,create}() Thomas Zimmermann 2020-02-07 8:41 ` Thomas Zimmermann 2020-02-07 10:33 ` Gerd Hoffmann 2020-02-07 10:33 ` Gerd Hoffmann 2020-02-07 10:50 ` Thomas Zimmermann 2020-02-07 10:50 ` Thomas Zimmermann 2020-02-07 12:31 ` Gerd Hoffmann 2020-02-07 12:31 ` Gerd Hoffmann 2020-02-07 13:37 ` Daniel Vetter [this message] 2020-02-07 13:37 ` Daniel Vetter 2020-02-07 14:00 ` Ville Syrjälä 2020-02-07 14:00 ` Ville Syrjälä 2020-02-07 14:02 ` Thomas Zimmermann 2020-02-07 14:02 ` Thomas Zimmermann 2020-02-07 16:26 ` Daniel Vetter 2020-02-07 16:26 ` Daniel Vetter 2020-02-07 14:36 ` Noralf Trønnes 2020-02-07 14:36 ` Noralf Trønnes 2020-02-07 16:25 ` Daniel Vetter 2020-02-07 16:25 ` Daniel Vetter 2020-02-10 10:28 ` kbuild test robot 2020-02-10 10:28 ` [PATCH 2/6] drm: Add drm_simple_encoder_{init, create}() kbuild test robot 2020-02-10 10:28 ` [PATCH 2/6] drm: Add drm_simple_encoder_{init,create}() kbuild test robot 2020-02-07 8:41 ` [PATCH 3/6] drm/ast: Use simple encoder Thomas Zimmermann 2020-02-07 8:41 ` Thomas Zimmermann 2020-02-07 8:41 ` [PATCH 4/6] drm/mgag200: " Thomas Zimmermann 2020-02-07 8:41 ` Thomas Zimmermann 2020-02-07 8:41 ` [PATCH 5/6] drm/qxl: " Thomas Zimmermann 2020-02-07 8:41 ` Thomas Zimmermann 2020-02-10 9:08 ` kbuild test robot 2020-02-10 9:08 ` kbuild test robot 2020-02-10 9:08 ` kbuild test robot 2020-02-07 8:41 ` [PATCH 6/6] drm/simple-pipe: " Thomas Zimmermann 2020-02-07 8:41 ` Thomas Zimmermann
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200207133720.GZ43062@phenom.ffwll.local \ --to=daniel@ffwll.ch \ --cc=airlied@linux.ie \ --cc=alexander.deucher@amd.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=emil.velikov@collabora.com \ --cc=maarten.lankhorst@linux.intel.com \ --cc=mripard@kernel.org \ --cc=noralf@tronnes.org \ --cc=sam@ravnborg.org \ --cc=spice-devel@lists.freedesktop.org \ --cc=tzimmermann@suse.de \ --cc=virtualization@lists.linux-foundation.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.