Many DRM drivers implement an encoder with an empty implementation. This patchset adds drm_simple_encoder_init() and drm_simple_encoder_create(), which can be used by drivers instead. Except for the destroy callback, the simple encoder's implementation is empty.
The patchset also converts 4 encoder instances to use the simple-encoder helpers. But there are at least 11 other drivers which can use the helper and I think I did not examine all drivers yet.
The patchset was smoke-tested on mgag200 by running the fbdev console and Gnome on X11.
Thomas Zimmermann (6): drm: Move initialization of encoder into an internal function drm: Add drm_simple_encoder_{init,create}() drm/ast: Use simple encoder drm/mgag200: Use simple encoder drm/qxl: Use simple encoder drm/simple-pipe: Use simple encoder
drivers/gpu/drm/ast/ast_drv.h | 6 +- drivers/gpu/drm/ast/ast_mode.c | 25 +--- drivers/gpu/drm/drm_encoder.c | 190 +++++++++++++++++++++--- drivers/gpu/drm/drm_simple_kms_helper.c | 8 +- drivers/gpu/drm/mgag200/mgag200_drv.h | 7 - drivers/gpu/drm/mgag200/mgag200_mode.c | 60 +------- drivers/gpu/drm/qxl/qxl_display.c | 17 +-- include/drm/drm_encoder.h | 10 ++ 8 files changed, 191 insertions(+), 132 deletions(-)
-- 2.25.0
Moving encoder init code into an internal function, so it can be reused.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_encoder.c | 78 +++++++++++++++++++++++------------ 1 file changed, 52 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c index e555281f43d4..ffe691a1bf34 100644 --- a/drivers/gpu/drm/drm_encoder.c +++ b/drivers/gpu/drm/drm_encoder.c @@ -91,6 +91,47 @@ void drm_encoder_unregister_all(struct drm_device *dev) } }
+static int __drm_encoder_init(struct drm_device *dev, + struct drm_encoder *encoder, + const struct drm_encoder_funcs *funcs, + int encoder_type, char *name) +{ + int ret; + + /* encoder index is used with 32-bit bitmasks */ + if (WARN_ON(dev->mode_config.num_encoder >= 32)) + return -EINVAL; + + ret = drm_mode_object_add(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER); + if (ret) + return ret; + + encoder->dev = dev; + encoder->encoder_type = encoder_type; + encoder->funcs = funcs; + if (name) { + encoder->name = name; + } else { + encoder->name = kasprintf(GFP_KERNEL, "%s-%d", + drm_encoder_enum_list[encoder_type].name, + encoder->base.id); + if (!encoder->name) { + ret = -ENOMEM; + goto out_put; + } + } + + INIT_LIST_HEAD(&encoder->bridge_chain); + list_add_tail(&encoder->head, &dev->mode_config.encoder_list); + encoder->index = dev->mode_config.num_encoder++; + +out_put: + if (ret) + drm_mode_object_unregister(dev, &encoder->base); + + return ret; +} + /** * drm_encoder_init - Init a preallocated encoder * @dev: drm device @@ -111,43 +152,28 @@ int drm_encoder_init(struct drm_device *dev, const struct drm_encoder_funcs *funcs, int encoder_type, const char *name, ...) { + char *namestr = NULL; int ret;
- /* encoder index is used with 32bit bitmasks */ - if (WARN_ON(dev->mode_config.num_encoder >= 32)) - return -EINVAL; - - ret = drm_mode_object_add(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER); - if (ret) - return ret; - - encoder->dev = dev; - encoder->encoder_type = encoder_type; - encoder->funcs = funcs; if (name) { va_list ap;
va_start(ap, name); - encoder->name = kvasprintf(GFP_KERNEL, name, ap); + namestr = kvasprintf(GFP_KERNEL, name, ap); va_end(ap); - } else { - encoder->name = kasprintf(GFP_KERNEL, "%s-%d", - drm_encoder_enum_list[encoder_type].name, - encoder->base.id); - } - if (!encoder->name) { - ret = -ENOMEM; - goto out_put; + if (!namestr) + return -ENOMEM; }
- INIT_LIST_HEAD(&encoder->bridge_chain); - list_add_tail(&encoder->head, &dev->mode_config.encoder_list); - encoder->index = dev->mode_config.num_encoder++; - -out_put: + ret = __drm_encoder_init(dev, encoder, funcs, encoder_type, namestr); if (ret) - drm_mode_object_unregister(dev, &encoder->base); + goto err_kfree; + + return 0;
+err_kfree: + if (name) + kfree(namestr); return ret; } EXPORT_SYMBOL(drm_encoder_init);
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 --- 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
+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, ...)
How about using
#define drm_simple_encoder_init(dev, type, name, ...) \ drm_encoder_init(dev, drm_simple_encoder_funcs_cleanup, type, name, __VA_ARGS__)
instead ?
cheers, Gerd
Hi Gerd
Am 07.02.20 um 11:33 schrieb Gerd Hoffmann:
+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, ...)
How about using
#define drm_simple_encoder_init(dev, type, name, ...) \ drm_encoder_init(dev, drm_simple_encoder_funcs_cleanup, type, name, __VA_ARGS__)
instead ?
I'd prefer a function call for aesthetic reasons and for not having to export the drm_simple_encoder_funcs_cleanup. drm_simple_encoder_create() is also a function and probably cannot be turned into a macro. So having a function for drm_simple_encoder_init seems consequent.
I guess you want to save a few lines in the implementation of drm_simple_encoder_init() (?) If so, I'd rather try to share more internal code among the various init and create functions.
Best regards Thomas
cheers, Gerd
How about using
#define drm_simple_encoder_init(dev, type, name, ...) \ drm_encoder_init(dev, drm_simple_encoder_funcs_cleanup, type, name, __VA_ARGS__)
instead ?
I guess you want to save a few lines in the implementation of drm_simple_encoder_init() (?) If so, I'd rather try to share more internal code among the various init and create functions.
Yes. You have the namestr stuff duplicated in all functions, and with a #define that goes away.
But maybe that can be simply be dropped? The drivers with a simple encoder seem to not care much about the name and just pass NULL ...
cheers, Gerd
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
On Fri, Feb 07, 2020 at 02:37:20PM +0100, Daniel Vetter wrote:
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.
I had some improvements for this stuff. Just re-sent the remainder.
Hi
Am 07.02.20 um 14:37 schrieb Daniel Vetter:
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 ...
How would such a helper roughly look like?
Best regards Thomas
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
On Fri, Feb 07, 2020 at 03:02:00PM +0100, Thomas Zimmermann wrote:
Hi
Am 07.02.20 um 14:37 schrieb Daniel Vetter:
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 ...
How would such a helper roughly look like?
Essentially same code, but stuff the helper into drm-kms-helper.ko, then make sure it still works. The separate kernel module makes sure that the drm core and helper stuff aren't too close friends with each another :-) Essentially like the simple display pipe helpers. -Daniel
Best regards Thomas
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
-- 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
Den 07.02.2020 09.41, skrev Thomas Zimmermann:
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
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
<snip>
+/**
- 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);
The encoder can outlive the devres if the device is unbound when userspace has open fds, so you can't use devm_ here.
Noralf.
- 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);
On Fri, Feb 07, 2020 at 03:36:49PM +0100, Noralf Trønnes wrote:
Den 07.02.2020 09.41, skrev Thomas Zimmermann:
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
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
<snip>
+/**
- 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);
The encoder can outlive the devres if the device is unbound when userspace has open fds, so you can't use devm_ here.
Ah yes great catch. Rule of thumb: Never use devm_ for any drm_* structure. It's wrong.
There's a todo somewhere to essentially create a drm_managed set of apis where the cleanup matches the right lifetime - we need to only free/release drm resource at drm_driver->release time. -Daniel
Noralf.
- 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);
Hi Thomas,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master] [also build test WARNING on v5.6-rc1 next-20200210] [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/Thomas-Zimmermann/drm-Provide-a-sim... base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9 reproduce: make htmldocs
If you fix the issue, kindly add following tag Reported-by: kbuild test robot lkp@intel.com
All warnings (new ones prefixed by >>):
include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'getprocattr' not described in 'security_list_options' include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'setprocattr' not described in 'security_list_options' include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'locked_down' not described in 'security_list_options' include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'perf_event_open' not described in 'security_list_options' include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'perf_event_alloc' not described in 'security_list_options' include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'perf_event_free' not described in 'security_list_options' include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'perf_event_read' not described in 'security_list_options' include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'perf_event_write' not described in 'security_list_options' drivers/usb/typec/bus.c:1: warning: 'typec_altmode_register_driver' not found drivers/usb/typec/bus.c:1: warning: 'typec_altmode_unregister_driver' not found drivers/usb/typec/class.c:1: warning: 'typec_altmode_unregister_notifier' not found drivers/usb/typec/class.c:1: warning: 'typec_altmode_register_notifier' not found include/linux/regulator/machine.h:196: warning: Function parameter or member 'max_uV_step' not described in 'regulation_constraints' include/linux/regulator/driver.h:223: warning: Function parameter or member 'resume' not described in 'regulator_ops' include/linux/skbuff.h:890: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff' include/linux/skbuff.h:890: warning: Function parameter or member 'list' not described in 'sk_buff' include/linux/skbuff.h:890: warning: Function parameter or member 'ip_defrag_offset' not described in 'sk_buff' include/linux/skbuff.h:890: warning: Function parameter or member 'skb_mstamp_ns' not described in 'sk_buff' include/linux/skbuff.h:890: warning: Function parameter or member '__cloned_offset' not described in 'sk_buff' include/linux/skbuff.h:890: warning: Function parameter or member 'head_frag' not described in 'sk_buff' include/linux/skbuff.h:890: warning: Function parameter or member '__pkt_type_offset' not described in 'sk_buff' include/linux/skbuff.h:890: warning: Function parameter or member 'encapsulation' not described in 'sk_buff' include/linux/skbuff.h:890: warning: Function parameter or member 'encap_hdr_csum' not described in 'sk_buff' include/linux/skbuff.h:890: warning: Function parameter or member 'csum_valid' not described in 'sk_buff' include/linux/skbuff.h:890: warning: Function parameter or member '__pkt_vlan_present_offset' not described in 'sk_buff' include/linux/skbuff.h:890: warning: Function parameter or member 'vlan_present' not described in 'sk_buff' include/linux/skbuff.h:890: warning: Function parameter or member 'csum_complete_sw' not described in 'sk_buff' include/linux/skbuff.h:890: warning: Function parameter or member 'csum_level' not described in 'sk_buff' include/linux/skbuff.h:890: warning: Function parameter or member 'inner_protocol_type' not described in 'sk_buff' include/linux/skbuff.h:890: warning: Function parameter or member 'remcsum_offload' not described in 'sk_buff' include/linux/skbuff.h:890: warning: Function parameter or member 'sender_cpu' not described in 'sk_buff' include/linux/skbuff.h:890: warning: Function parameter or member 'reserved_tailroom' not described in 'sk_buff' include/linux/skbuff.h:890: warning: Function parameter or member 'inner_ipproto' not described in 'sk_buff' include/net/sock.h:232: warning: Function parameter or member 'skc_addrpair' not described in 'sock_common' include/net/sock.h:232: warning: Function parameter or member 'skc_portpair' not described in 'sock_common' include/net/sock.h:232: warning: Function parameter or member 'skc_ipv6only' not described in 'sock_common' include/net/sock.h:232: warning: Function parameter or member 'skc_net_refcnt' not described in 'sock_common' include/net/sock.h:232: warning: Function parameter or member 'skc_v6_daddr' not described in 'sock_common' include/net/sock.h:232: warning: Function parameter or member 'skc_v6_rcv_saddr' not described in 'sock_common' include/net/sock.h:232: warning: Function parameter or member 'skc_cookie' not described in 'sock_common' include/net/sock.h:232: warning: Function parameter or member 'skc_listener' not described in 'sock_common' include/net/sock.h:232: warning: Function parameter or member 'skc_tw_dr' not described in 'sock_common' include/net/sock.h:232: warning: Function parameter or member 'skc_rcv_wnd' not described in 'sock_common' include/net/sock.h:232: warning: Function parameter or member 'skc_tw_rcv_nxt' not described in 'sock_common' include/net/sock.h:498: warning: Function parameter or member 'sk_rx_skb_cache' not described in 'sock' include/net/sock.h:498: warning: Function parameter or member 'sk_wq_raw' not described in 'sock' include/net/sock.h:498: warning: Function parameter or member 'tcp_rtx_queue' not described in 'sock' include/net/sock.h:498: warning: Function parameter or member 'sk_tx_skb_cache' not described in 'sock' include/net/sock.h:498: warning: Function parameter or member 'sk_route_forced_caps' not described in 'sock' include/net/sock.h:498: warning: Function parameter or member 'sk_txtime_report_errors' not described in 'sock' include/net/sock.h:498: warning: Function parameter or member 'sk_validate_xmit_skb' not described in 'sock' include/net/sock.h:498: warning: Function parameter or member 'sk_bpf_storage' not described in 'sock' include/net/sock.h:2444: warning: Function parameter or member 'tcp_rx_skb_cache_key' not described in 'DECLARE_STATIC_KEY_FALSE' include/net/sock.h:2444: warning: Excess function parameter 'sk' description in 'DECLARE_STATIC_KEY_FALSE' include/net/sock.h:2444: warning: Excess function parameter 'skb' description in 'DECLARE_STATIC_KEY_FALSE' include/linux/netdevice.h:2100: warning: Function parameter or member 'gso_partial_features' not described in 'net_device' include/linux/netdevice.h:2100: warning: Function parameter or member 'l3mdev_ops' not described in 'net_device' include/linux/netdevice.h:2100: warning: Function parameter or member 'xfrmdev_ops' not described in 'net_device' include/linux/netdevice.h:2100: warning: Function parameter or member 'tlsdev_ops' not described in 'net_device' include/linux/netdevice.h:2100: warning: Function parameter or member 'name_assign_type' not described in 'net_device' include/linux/netdevice.h:2100: warning: Function parameter or member 'ieee802154_ptr' not described in 'net_device' include/linux/netdevice.h:2100: warning: Function parameter or member 'mpls_ptr' not described in 'net_device' include/linux/netdevice.h:2100: warning: Function parameter or member 'xdp_prog' not described in 'net_device' include/linux/netdevice.h:2100: warning: Function parameter or member 'gro_flush_timeout' not described in 'net_device' include/linux/netdevice.h:2100: warning: Function parameter or member 'xdp_bulkq' not described in 'net_device' include/linux/netdevice.h:2100: warning: Function parameter or member 'xps_cpus_map' not described in 'net_device' include/linux/netdevice.h:2100: warning: Function parameter or member 'xps_rxqs_map' not described in 'net_device' include/linux/netdevice.h:2100: warning: Function parameter or member 'qdisc_hash' not described in 'net_device' include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising' not described in 'phylink_link_state' include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising' not described in 'phylink_link_state' drivers/infiniband/core/umem_odp.c:161: warning: Function parameter or member 'ops' not described in 'ib_umem_odp_alloc_child' drivers/infiniband/core/umem_odp.c:211: warning: Function parameter or member 'ops' not described in 'ib_umem_odp_get' drivers/infiniband/ulp/iser/iscsi_iser.h:401: warning: Function parameter or member 'all_list' not described in 'iser_fr_desc' drivers/infiniband/ulp/iser/iscsi_iser.h:415: warning: Function parameter or member 'all_list' not described in 'iser_fr_pool' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:148: warning: Function parameter or member 'rsvd0' not described in 'opa_vesw_info' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:148: warning: Function parameter or member 'rsvd1' not described in 'opa_vesw_info' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:148: warning: Function parameter or member 'rsvd2' not described in 'opa_vesw_info' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:148: warning: Function parameter or member 'rsvd3' not described in 'opa_vesw_info' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:148: warning: Function parameter or member 'rsvd4' not described in 'opa_vesw_info' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:205: warning: Function parameter or member 'rsvd0' not described in 'opa_per_veswport_info' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:205: warning: Function parameter or member 'rsvd1' not described in 'opa_per_veswport_info' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:205: warning: Function parameter or member 'rsvd2' not described in 'opa_per_veswport_info' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:205: warning: Function parameter or member 'rsvd3' not described in 'opa_per_veswport_info' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:263: warning: Function parameter or member 'tbl_entries' not described in 'opa_veswport_mactable' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:342: warning: Function parameter or member 'reserved' not described in 'opa_veswport_summary_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd0' not described in 'opa_veswport_error_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd1' not described in 'opa_veswport_error_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd2' not described in 'opa_veswport_error_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd3' not described in 'opa_veswport_error_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd4' not described in 'opa_veswport_error_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd5' not described in 'opa_veswport_error_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd6' not described in 'opa_veswport_error_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd7' not described in 'opa_veswport_error_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd8' not described in 'opa_veswport_error_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd9' not described in 'opa_veswport_error_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:460: warning: Function parameter or member 'reserved' not described in 'opa_vnic_vema_mad' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:485: warning: Function parameter or member 'reserved' not described in 'opa_vnic_notice_attr' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:500: warning: Function parameter or member 'reserved' not described in 'opa_vnic_vema_mad_trap' include/linux/input/sparse-keymap.h:43: warning: Function parameter or member 'sw' not described in 'key_entry' drivers/gpu/drm/drm_encoder.c:202: warning: Excess function parameter 'funcs' description in 'drm_simple_encoder_init'
drivers/gpu/drm/drm_encoder.c:203: warning: Function parameter or member 'encoder' not described in 'drm_simple_encoder_init'
drivers/gpu/drm/drm_encoder.c:203: warning: Excess function parameter 'funcs' description in 'drm_simple_encoder_init' include/drm/drm_modeset_helper_vtables.h:1052: warning: Function parameter or member 'prepare_writeback_job' not described in 'drm_connector_helper_funcs' include/drm/drm_modeset_helper_vtables.h:1052: warning: Function parameter or member 'cleanup_writeback_job' not described in 'drm_connector_helper_funcs' drivers/gpu/drm/bridge/panel.c:303: warning: Function parameter or member 'bridge' not described in 'drm_panel_bridge_connector' include/drm/drm_dp_mst_helper.h:162: warning: Function parameter or member 'fec_capable' not described in 'drm_dp_mst_port' include/drm/gpu_scheduler.h:103: warning: Function parameter or member 'priority' not described in 'drm_sched_entity' drivers/gpu/drm/i915/i915_vma.h:1: warning: 'Virtual Memory Address' not found drivers/gpu/drm/i915/i915_gem_gtt.c:1: warning: 'Global GTT views' not found include/linux/host1x.h:66: warning: Function parameter or member 'parent' not described in 'host1x_client' include/linux/host1x.h:66: warning: Function parameter or member 'usecount' not described in 'host1x_client' include/linux/host1x.h:66: warning: Function parameter or member 'lock' not described in 'host1x_client' include/net/cfg80211.h:1189: warning: Function parameter or member 'txpwr' not described in 'station_parameters' include/net/mac80211.h:4081: warning: Function parameter or member 'sta_set_txpwr' not described in 'ieee80211_ops' include/net/mac80211.h:2036: warning: Function parameter or member 'txpwr' not described in 'ieee80211_sta' kernel/sched/fair.c:3526: warning: Excess function parameter 'flags' description in 'attach_entity_load_avg' Documentation/admin-guide/acpi/fan_performance_states.rst:21: WARNING: Literal block ends without a blank line; unexpected unindent. Documentation/admin-guide/acpi/fan_performance_states.rst:41: WARNING: Literal block expected; none found. Documentation/admin-guide/perf/imx-ddr.rst:47: WARNING: Unexpected indentation. Documentation/x86/index.rst:7: WARNING: toctree contains reference to nonexisting document 'x86/intel_mpx' Documentation/admin-guide/bootconfig.rst:26: WARNING: Literal block expected; none found. Documentation/admin-guide/hw-vuln/tsx_async_abort.rst:142: WARNING: duplicate label virt_mechanism, other instance in Documentation/admin-guide/hw-vuln/mds.rst drivers/message/fusion/mptbase.c:5057: WARNING: Definition list ends without a blank line; unexpected unindent. include/uapi/linux/firewire-cdev.h:312: WARNING: Inline literal start-string without end-string. drivers/firewire/core-transaction.c:606: WARNING: Inline strong start-string without end-string. Documentation/driver-api/gpio/driver.rst:425: WARNING: Unexpected indentation. Documentation/driver-api/gpio/driver.rst:423: WARNING: Inline emphasis start-string without end-string. Documentation/driver-api/gpio/driver.rst:427: WARNING: Block quote ends without a blank line; unexpected unindent. Documentation/driver-api/gpio/driver.rst:429: WARNING: Inline emphasis start-string without end-string. Documentation/driver-api/gpio/driver.rst:429: WARNING: Inline emphasis start-string without end-string. Documentation/driver-api/gpio/driver.rst:429: WARNING: Inline emphasis start-string without end-string. Documentation/driver-api/gpio/driver.rst:433: WARNING: Inline emphasis start-string without end-string. Documentation/driver-api/gpio/driver.rst:446: WARNING: Unexpected indentation. Documentation/driver-api/gpio/driver.rst:440: WARNING: Inline emphasis start-string without end-string. Documentation/driver-api/gpio/driver.rst:440: WARNING: Inline emphasis start-string without end-string. Documentation/driver-api/gpio/driver.rst:447: WARNING: Block quote ends without a blank line; unexpected unindent. Documentation/driver-api/gpio/driver.rst:449: WARNING: Definition list ends without a blank line; unexpected unindent. Documentation/driver-api/gpio/driver.rst:462: WARNING: Unexpected indentation. Documentation/driver-api/gpio/driver.rst:460: WARNING: Inline emphasis start-string without end-string. Documentation/driver-api/gpio/driver.rst:462: WARNING: Inline emphasis start-string without end-string. Documentation/driver-api/gpio/driver.rst:465: WARNING: Block quote ends without a blank line; unexpected unindent. Documentation/driver-api/gpio/driver.rst:467: WARNING: Inline emphasis start-string without end-string. Documentation/driver-api/gpio/driver.rst:467: WARNING: Inline emphasis start-string without end-string. Documentation/driver-api/gpio/driver.rst:467: WARNING: Inline emphasis start-string without end-string. Documentation/driver-api/gpio/driver.rst:471: WARNING: Inline emphasis start-string without end-string. Documentation/driver-api/gpio/driver.rst:478: WARNING: Inline emphasis start-string without end-string. include/linux/spi/spi.h:399: WARNING: Unexpected indentation. Documentation/power/index.rst:7: WARNING: toctree contains reference to nonexisting document 'power/interface' Documentation/power/pm_qos_interface.rst:12: WARNING: Unexpected indentation. Documentation/admin-guide/ras.rst:358: WARNING: Definition list ends without a blank line; unexpected unindent. Documentation/admin-guide/ras.rst:358: WARNING: Definition list ends without a blank line; unexpected unindent. Documentation/admin-guide/ras.rst:363: WARNING: Definition list ends without a blank line; unexpected unindent. Documentation/admin-guide/ras.rst:363: WARNING: Definition list ends without a blank line; unexpected unindent. include/linux/devfreq.h:156: WARNING: Inline emphasis start-string without end-string. include/linux/devfreq.h:261: WARNING: Inline emphasis start-string without end-string. include/linux/devfreq.h:281: WARNING: Inline emphasis start-string without end-string. Documentation/driver-api/dmaengine/client.rst:203: WARNING: Unexpected indentation. Documentation/driver-api/dmaengine/client.rst:204: WARNING: Block quote ends without a blank line; unexpected unindent. Documentation/driver-api/dmaengine/client.rst:210: WARNING: Unexpected indentation. Documentation/driver-api/dmaengine/client.rst:211: WARNING: Block quote ends without a blank line; unexpected unindent. Documentation/driver-api/dmaengine/client.rst:220: WARNING: Unexpected indentation. Documentation/driver-api/dmaengine/client.rst:221: WARNING: Block quote ends without a blank line; unexpected unindent. Documentation/driver-api/dmaengine/client.rst:229: WARNING: Unexpected indentation. Documentation/driver-api/dmaengine/client.rst:230: WARNING: Block quote ends without a blank line; unexpected unindent. Documentation/driver-api/dmaengine/provider.rst:270: WARNING: Unexpected indentation. Documentation/driver-api/dmaengine/provider.rst:273: WARNING: Block quote ends without a blank line; unexpected unindent. Documentation/driver-api/dmaengine/provider.rst:288: WARNING: Unexpected indentation. Documentation/driver-api/dmaengine/provider.rst:290: WARNING: Block quote ends without a blank line; unexpected unindent. Documentation/driver-api/driver-model/driver.rst:215: WARNING: Inline emphasis start-string without end-string. Documentation/driver-api/driver-model/driver.rst:215: WARNING: Inline emphasis start-string without end-string. Documentation/filesystems/fuse.rst:2: WARNING: Explicit markup ends without a blank line; unexpected unindent. Documentation/filesystems/ubifs-authentication.rst:94: WARNING: Inline interpreted text or phrase reference start-string without end-string. Documentation/trace/events.rst:589: WARNING: Definition list ends without a blank line; unexpected unindent. Documentation/trace/events.rst:620: WARNING: Inline emphasis start-string without end-string. Documentation/trace/events.rst:623: WARNING: Inline emphasis start-string without end-string. Documentation/trace/events.rst:626: WARNING: Inline emphasis start-string without end-string. Documentation/trace/events.rst:703: WARNING: Inline emphasis start-string without end-string. Documentation/trace/events.rst:697: WARNING: Inline emphasis start-string without end-string. Documentation/trace/events.rst:722: WARNING: Inline emphasis start-string without end-string. Documentation/trace/events.rst:775: WARNING: Definition list ends without a blank line; unexpected unindent. Documentation/trace/events.rst:814: WARNING: Inline emphasis start-string without end-string. Documentation/trace/events.rst:817: WARNING: Inline emphasis start-string without end-string. Documentation/trace/events.rst:820: WARNING: Inline emphasis start-string without end-string. Documentation/trace/events.rst:823: WARNING: Inline emphasis start-string without end-string. Documentation/trace/events.rst:826: WARNING: Inline emphasis start-string without end-string. Documentation/trace/events.rst:829: WARNING: Inline emphasis start-string without end-string. Documentation/trace/events.rst:832: WARNING: Inline emphasis start-string without end-string. Documentation/trace/events.rst:844: WARNING: Unexpected indentation. Documentation/trace/events.rst:845: WARNING: Block quote ends without a blank line; unexpected unindent. Documentation/trace/events.rst:849: WARNING: Unexpected indentation. Documentation/trace/events.rst:850: WARNING: Block quote ends without a blank line; unexpected unindent. Documentation/trace/events.rst:883: WARNING: Inline emphasis start-string without end-string. Documentation/trace/events.rst:886: WARNING: Inline emphasis start-string without end-string. Documentation/trace/events.rst:889: WARNING: Inline emphasis start-string without end-string. Documentation/trace/events.rst:895: WARNING: Bullet list ends without a blank line; unexpected unindent. Documentation/trace/events.rst:895: WARNING: Inline emphasis start-string without end-string. Documentation/trace/events.rst:968: WARNING: Inline emphasis start-string without end-string. fs/inode.c:1608: WARNING: Inline emphasis start-string without end-string. fs/inode.c:1608: WARNING: Inline emphasis start-string without end-string. fs/inode.c:1614: WARNING: Inline emphasis start-string without end-string. fs/seq_file.c:40: WARNING: Inline strong start-string without end-string.
vim +203 drivers/gpu/drm/drm_encoder.c
184 185 /** 186 * drm_simple_encoder_init - Init a preallocated encoder 187 * @dev: drm device 188 * @funcs: callbacks for this encoder 189 * @encoder_type: user visible type of the encoder 190 * @name: printf style format string for the encoder name, or NULL 191 * for default name 192 * 193 * Initialises a preallocated encoder that has no further functionality. The 194 * encoder will be released automatically. 195 * 196 * Returns: 197 * Zero on success, error code on failure. 198 */ 199 int drm_simple_encoder_init(struct drm_device *dev, 200 struct drm_encoder *encoder, 201 int encoder_type, const char *name, ...) 202 {
203 char *namestr = NULL;
204 int ret; 205 206 if (name) { 207 va_list ap; 208 209 va_start(ap, name); 210 namestr = kvasprintf(GFP_KERNEL, name, ap); 211 va_end(ap); 212 if (!namestr) 213 return -ENOMEM; 214 } 215 216 ret = __drm_encoder_init(dev, encoder, 217 &drm_simple_encoder_funcs_cleanup, 218 encoder_type, namestr); 219 if (ret) 220 goto err_kfree; 221 222 return 0; 223 224 err_kfree: 225 if (name) 226 kfree(namestr); 227 return ret; 228 } 229 EXPORT_SYMBOL(drm_simple_encoder_init); 230
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
The ast driver uses an empty implementation for its encoder. Replace the code with the generic simple encoder.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/ast/ast_drv.h | 6 +----- drivers/gpu/drm/ast/ast_mode.c | 25 ++++++++----------------- 2 files changed, 9 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h index f5d8780776ae..656d591b154b 100644 --- a/drivers/gpu/drm/ast/ast_drv.h +++ b/drivers/gpu/drm/ast/ast_drv.h @@ -121,6 +121,7 @@ struct ast_private { unsigned int next_index; } cursor;
+ struct drm_encoder encoder; struct drm_plane primary_plane; struct drm_plane cursor_plane;
@@ -238,13 +239,8 @@ struct ast_crtc { u8 offset_x, offset_y; };
-struct ast_encoder { - struct drm_encoder base; -}; - #define to_ast_crtc(x) container_of(x, struct ast_crtc, base) #define to_ast_connector(x) container_of(x, struct ast_connector, base) -#define to_ast_encoder(x) container_of(x, struct ast_encoder, base)
struct ast_vbios_stdtable { u8 misc; diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 562ea6d9df13..60facaa152ac 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -968,28 +968,19 @@ static int ast_crtc_init(struct drm_device *dev) * Encoder */
-static void ast_encoder_destroy(struct drm_encoder *encoder) -{ - drm_encoder_cleanup(encoder); - kfree(encoder); -} - -static const struct drm_encoder_funcs ast_enc_funcs = { - .destroy = ast_encoder_destroy, -}; - static int ast_encoder_init(struct drm_device *dev) { - struct ast_encoder *ast_encoder; + struct ast_private *ast = dev->dev_private; + struct drm_encoder *encoder = &ast->encoder; + int ret;
- ast_encoder = kzalloc(sizeof(struct ast_encoder), GFP_KERNEL); - if (!ast_encoder) - return -ENOMEM; + ret = drm_simple_encoder_init(dev, encoder, DRM_MODE_ENCODER_DAC, + NULL); + if (ret) + return ret;
- drm_encoder_init(dev, &ast_encoder->base, &ast_enc_funcs, - DRM_MODE_ENCODER_DAC, NULL); + encoder->possible_crtcs = 1;
- ast_encoder->base.possible_crtcs = 1; return 0; }
The mgag200 driver uses an empty implementation for its encoder. Replace the code with the generic simple encoder.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/mgag200/mgag200_drv.h | 7 --- drivers/gpu/drm/mgag200/mgag200_mode.c | 60 +------------------------- 2 files changed, 2 insertions(+), 65 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index aa32aad222c2..9bb9e8e14539 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -95,7 +95,6 @@ #define MATROX_DPMS_CLEARED (-1)
#define to_mga_crtc(x) container_of(x, struct mga_crtc, base) -#define to_mga_encoder(x) container_of(x, struct mga_encoder, base) #define to_mga_connector(x) container_of(x, struct mga_connector, base)
struct mga_crtc { @@ -110,12 +109,6 @@ struct mga_mode_info { struct mga_crtc *crtc; };
-struct mga_encoder { - struct drm_encoder base; - int last_dpms; -}; - - struct mga_i2c_chan { struct i2c_adapter adapter; struct drm_device *dev; diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 62a8e9ccb16d..d8f1a552f8ac 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1449,72 +1449,16 @@ static void mga_crtc_init(struct mga_device *mdev) drm_crtc_helper_add(&mga_crtc->base, &mga_helper_funcs); }
-/* - * The encoder comes after the CRTC in the output pipeline, but before - * the connector. It's responsible for ensuring that the digital - * stream is appropriately converted into the output format. Setup is - * very simple in this case - all we have to do is inform qemu of the - * colour depth in order to ensure that it displays appropriately - */ - -/* - * These functions are analagous to those in the CRTC code, but are intended - * to handle any encoder-specific limitations - */ -static void mga_encoder_mode_set(struct drm_encoder *encoder, - struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode) -{ - -} - -static void mga_encoder_dpms(struct drm_encoder *encoder, int state) -{ - return; -} - -static void mga_encoder_prepare(struct drm_encoder *encoder) -{ -} - -static void mga_encoder_commit(struct drm_encoder *encoder) -{ -} - -static void mga_encoder_destroy(struct drm_encoder *encoder) -{ - struct mga_encoder *mga_encoder = to_mga_encoder(encoder); - drm_encoder_cleanup(encoder); - kfree(mga_encoder); -} - -static const struct drm_encoder_helper_funcs mga_encoder_helper_funcs = { - .dpms = mga_encoder_dpms, - .mode_set = mga_encoder_mode_set, - .prepare = mga_encoder_prepare, - .commit = mga_encoder_commit, -}; - -static const struct drm_encoder_funcs mga_encoder_encoder_funcs = { - .destroy = mga_encoder_destroy, -}; - static struct drm_encoder *mga_encoder_init(struct drm_device *dev) { struct drm_encoder *encoder; - struct mga_encoder *mga_encoder;
- mga_encoder = kzalloc(sizeof(struct mga_encoder), GFP_KERNEL); - if (!mga_encoder) + encoder = drm_simple_encoder_create(dev, DRM_MODE_ENCODER_DAC, NULL); + if (IS_ERR(encoder)) return NULL;
- encoder = &mga_encoder->base; encoder->possible_crtcs = 0x1;
- drm_encoder_init(dev, encoder, &mga_encoder_encoder_funcs, - DRM_MODE_ENCODER_DAC, NULL); - drm_encoder_helper_add(encoder, &mga_encoder_helper_funcs); - return encoder; }
The qxl driver uses an empty implementation for its encoder. Replace the code with the generic simple encoder.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/qxl/qxl_display.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c index ab4f8dd00400..fc71f7d9436a 100644 --- a/drivers/gpu/drm/qxl/qxl_display.c +++ b/drivers/gpu/drm/qxl/qxl_display.c @@ -1007,9 +1007,6 @@ static struct drm_encoder *qxl_best_encoder(struct drm_connector *connector) return &qxl_output->enc; }
-static const struct drm_encoder_helper_funcs qxl_enc_helper_funcs = { -}; - static const struct drm_connector_helper_funcs qxl_connector_helper_funcs = { .get_modes = qxl_conn_get_modes, .mode_valid = qxl_conn_mode_valid, @@ -1059,15 +1056,6 @@ static const struct drm_connector_funcs qxl_connector_funcs = { .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, };
-static void qxl_enc_destroy(struct drm_encoder *encoder) -{ - drm_encoder_cleanup(encoder); -} - -static const struct drm_encoder_funcs qxl_enc_funcs = { - .destroy = qxl_enc_destroy, -}; - static int qxl_mode_create_hotplug_mode_update_property(struct qxl_device *qdev) { if (qdev->hotplug_mode_update_property) @@ -1098,15 +1086,14 @@ static int qdev_output_init(struct drm_device *dev, int num_output) drm_connector_init(dev, &qxl_output->base, &qxl_connector_funcs, DRM_MODE_CONNECTOR_VIRTUAL);
- drm_encoder_init(dev, &qxl_output->enc, &qxl_enc_funcs, - DRM_MODE_ENCODER_VIRTUAL, NULL); + drm_simple_encoder_init(dev, qxl_output->enc, DRM_MODE_ENCODER_VIRTUAL, + NULL);
/* we get HPD via client monitors config */ connector->polled = DRM_CONNECTOR_POLL_HPD; encoder->possible_crtcs = 1 << num_output; drm_connector_attach_encoder(&qxl_output->base, &qxl_output->enc); - drm_encoder_helper_add(encoder, &qxl_enc_helper_funcs); drm_connector_helper_add(connector, &qxl_connector_helper_funcs);
drm_object_attach_property(&connector->base,
Hi Thomas,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master] [also build test ERROR on v5.6-rc1 next-20200210] [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/Thomas-Zimmermann/drm-Provide-a-sim... base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9 config: x86_64-randconfig-f002-20200210 (attached as .config) compiler: gcc-7 (Debian 7.5.0-3) 7.5.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/qxl/qxl_display.c: In function 'qdev_output_init':
drivers/gpu/drm/qxl/qxl_display.c:1103:31: error: incompatible type for argument 2 of 'drm_simple_encoder_init'
drm_simple_encoder_init(dev, qxl_output->enc, DRM_MODE_ENCODER_VIRTUAL, ^~~~~~~~~~ In file included from include/drm/drm_modeset_helper_vtables.h:33:0, from include/drm/drm_atomic_helper.h:32, from drivers/gpu/drm/qxl/qxl_display.c:30: include/drm/drm_encoder.h:194:5: note: expected 'struct drm_encoder *' but argument is of type 'struct drm_encoder' int drm_simple_encoder_init(struct drm_device *dev, ^~~~~~~~~~~~~~~~~~~~~~~
vim +/drm_simple_encoder_init +1103 drivers/gpu/drm/qxl/qxl_display.c
1084 1085 static int qdev_output_init(struct drm_device *dev, int num_output) 1086 { 1087 struct qxl_device *qdev = dev->dev_private; 1088 struct qxl_output *qxl_output; 1089 struct drm_connector *connector; 1090 struct drm_encoder *encoder; 1091 1092 qxl_output = kzalloc(sizeof(struct qxl_output), GFP_KERNEL); 1093 if (!qxl_output) 1094 return -ENOMEM; 1095 1096 qxl_output->index = num_output; 1097 1098 connector = &qxl_output->base; 1099 encoder = &qxl_output->enc; 1100 drm_connector_init(dev, &qxl_output->base, 1101 &qxl_connector_funcs, DRM_MODE_CONNECTOR_VIRTUAL); 1102
1103 drm_simple_encoder_init(dev, qxl_output->enc, DRM_MODE_ENCODER_VIRTUAL,
1104 NULL); 1105 1106 /* we get HPD via client monitors config */ 1107 connector->polled = DRM_CONNECTOR_POLL_HPD; 1108 encoder->possible_crtcs = 1 << num_output; 1109 drm_connector_attach_encoder(&qxl_output->base, 1110 &qxl_output->enc); 1111 drm_connector_helper_add(connector, &qxl_connector_helper_funcs); 1112 1113 drm_object_attach_property(&connector->base, 1114 qdev->hotplug_mode_update_property, 0); 1115 drm_object_attach_property(&connector->base, 1116 dev->mode_config.suggested_x_property, 0); 1117 drm_object_attach_property(&connector->base, 1118 dev->mode_config.suggested_y_property, 0); 1119 return 0; 1120 } 1121
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
The simple-pipe helpers use an empty implementation for the encoder. Replace the code with the generic simple encoder.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_simple_kms_helper.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c index 15fb516ae2d8..e16606b3ee20 100644 --- a/drivers/gpu/drm/drm_simple_kms_helper.c +++ b/drivers/gpu/drm/drm_simple_kms_helper.c @@ -28,10 +28,6 @@ * encoder drivers. */
-static const struct drm_encoder_funcs drm_simple_kms_encoder_funcs = { - .destroy = drm_encoder_cleanup, -}; - static enum drm_mode_status drm_simple_kms_crtc_mode_valid(struct drm_crtc *crtc, const struct drm_display_mode *mode) @@ -288,8 +284,8 @@ int drm_simple_display_pipe_init(struct drm_device *dev, return ret;
encoder->possible_crtcs = drm_crtc_mask(crtc); - ret = drm_encoder_init(dev, encoder, &drm_simple_kms_encoder_funcs, - DRM_MODE_ENCODER_NONE, NULL); + ret = drm_simple_encoder_init(dev, encoder, DRM_MODE_ENCODER_NONE, + NULL); if (ret || !connector) return ret;
dri-devel@lists.freedesktop.org