On Sat, Feb 11, 2017 at 8:48 PM, Noralf Trønnes <noralf@xxxxxxxxxxx> wrote:
> tinydrm provides helpers for very simple displays that can use
> CMA backed framebuffers and need flushing on changes.
> +/**
> + * tinydrm_gem_cma_free_object - Free resources associated with a CMA GEM
> + * object
Keep on one line?
> +const struct file_operations tinydrm_fops = {
> + .owner = THIS_MODULE,
Do we still need this?
> +static int tinydrm_init(struct device *parent, struct tinydrm_device *tdev,
> + const struct drm_framebuffer_funcs *fb_funcs,
> + struct drm_driver *driver)
> +{
> + struct drm_device *drm;
> +
> + mutex_init(&tdev->dirty_lock);
> + tdev->fb_funcs = fb_funcs;
> +
> + /*
> + * We don't embed drm_device, because that prevent us from using
> + * devm_kzalloc() to allocate tinydrm_device in the driver since
> + * drm_dev_unref() frees the structure. The devm_ functions provide
"devm_ functions" -> "devm_kzalloc()" ?
Otherwise what else do you refer to and why here?
> + * for easy error handling.
> + */
> +static int tinydrm_register(struct tinydrm_device *tdev)
> +{
> + struct drm_device *drm = tdev->drm;
> + int bpp = drm->mode_config.preferred_depth;
> + struct drm_fbdev_cma *fbdev;
> + int ret;
> +
> + ret = drm_dev_register(tdev->drm, 0);
> + if (ret)
> + return ret;
> +
> + fbdev = drm_fbdev_cma_init_with_funcs(drm, bpp ? bpp : 32,
> + drm->mode_config.num_connector,
> + tdev->fb_funcs);
> + if (IS_ERR(fbdev))
> + DRM_ERROR("Failed to initialize fbdev: %ld\n", PTR_ERR(fbdev));
> + else
> + tdev->fbdev_cma = fbdev;
Apparently I missed previous round of reviews, can you just put in
short words why error is not propagated here to the caller?
> +
> + return 0;
> +}
> +/**
> + * tinydrm_display_pipe_init - Initialize display pipe
> + * @tdev: tinydrm device
> + * @funcs: Display pipe functions
> + * @connector_type: Connector type
> + * @formats: Array of supported formats (DRM_FORMAT\_\*)
DRM_FORMAT_* ?
> + * @format_count: Number of elements in @formats
> + * @mode: Supported mode
> + * @rotation: Initial @mode rotation in degrees Counter Clock Wise
> + *
> + * This function sets up a &drm_simple_display_pipe with a &drm_connector that
> + * has one fixed &drm_display_mode which is rotated according to @rotation.
> + *
> + * Returns:
> + * Zero on success, negative error code on failure.
> + */
> +{
> + struct drm_device *drm = tdev->drm;
> + struct drm_display_mode *mode_copy;
> + struct drm_connector *connector;
> + int ret;
> + connector = tinydrm_connector_create(drm, mode_copy, connector_type);
> + if (IS_ERR(connector))
> + return PTR_ERR(connector);
> +
> + ret = drm_simple_display_pipe_init(drm, &tdev->pipe, funcs, formats,
> + format_count, connector);
> + if (ret)
> + return ret;
> +
> + return 0;
return drm_... ?
> +}
> +EXPORT_SYMBOL(tinydrm_display_pipe_init);
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
