Den 05.05.2016 19:03, skrev Daniel Vetter:
> On Thu, May 05, 2016 at 03:24:34PM +0200, Noralf Trønnes wrote:
>> Add function to create a simple connector for a panel.
>>
>> Signed-off-by: Noralf Trønnes <noralf at tronnes.org>
> Like in the previous patch please also add a new section for the panel
> helpers to gpu.tmpl. I don't think this needs an overview section, it's so
> simple. But adding some cross references from the drm_panel.c kerneldoc to
> this and back would be real good.
drm_panel.c doesn't have any documentation and the header file has only
the drm_panel_funcs struct documented, not hooked up to gpu.tmpl.
I can make a patch documenting the functions, it looks fairly straight
forward, but I have no idea what to put in the DOC: section, except an
xref to this helper :-)
Noralf.
>> ---
>> drivers/gpu/drm/Makefile | 1 +
>> drivers/gpu/drm/drm_panel_helper.c | 117
>> +++++++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/panel/Kconfig | 7 +++
>> include/drm/drm_panel_helper.h | 20 +++++++
>> 4 files changed, 145 insertions(+)
>> create mode 100644 drivers/gpu/drm/drm_panel_helper.c
>> create mode 100644 include/drm/drm_panel_helper.h
>>
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index 7e99923..3f3696c 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -18,6 +18,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o
>> drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
>> drm-$(CONFIG_PCI) += ati_pcigart.o
>> drm-$(CONFIG_DRM_PANEL) += drm_panel.o
>> +drm-$(CONFIG_DRM_PANEL_HELPER) += drm_panel_helper.o
>> drm-$(CONFIG_OF) += drm_of.o
>> drm-$(CONFIG_AGP) += drm_agpsupport.o
>>
>> diff --git a/drivers/gpu/drm/drm_panel_helper.c
>> b/drivers/gpu/drm/drm_panel_helper.c
>> new file mode 100644
>> index 0000000..5d8b623
>> --- /dev/null
>> +++ b/drivers/gpu/drm/drm_panel_helper.c
>> @@ -0,0 +1,117 @@
>> +/*
>> + * Copyright (C) 2016 Noralf Trønnes
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/slab.h>
>> +
>> +#include <drm/drmP.h>
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_crtc_helper.h>
>> +#include <drm/drm_panel.h>
>> +
>> +struct drm_panel_connector {
>> + struct drm_connector base;
>> + struct drm_panel *panel;
>> +};
>> +
>> +static inline struct drm_panel_connector *
>> +to_drm_panel_connector(struct drm_connector *connector)
>> +{
>> + return container_of(connector, struct drm_panel_connector, base);
>> +}
>> +
>> +static int drm_panel_connector_get_modes(struct drm_connector *connector)
>> +{
>> + return drm_panel_get_modes(to_drm_panel_connector(connector)->panel);
>> +}
>> +
>> +static struct drm_encoder *
>> +drm_panel_connector_best_encoder(struct drm_connector *connector)
>> +{
>> + return drm_encoder_find(connector->dev, connector->encoder_ids[0]);
>> +}
> As mentioned in my previous review, this function would be extremely
> useful to rid tons of drivers of boilerplate - most drm_connector only
> support exactly 1 encoder, statically determined at driver init time.
>
> Please put this helper into the atomic helper library, and maybe call it
> something like
>
> drm_atomic_helper_best_encoder.
>
> To make sure it's never abused by accident please also add a
>
> WARN_ON(connector->encoder_ids[1]);
>
> to make sure that there's really only just one encoder for this connector.
>
> If you're bored you can also go through drivers and use that everywhere it
> fits, it would result in a _lot_ of deleted code. But also needs quite a
> bit of audit work ...
>
> Otherwise lgtm, but please ask Thierry for an ack as the panel maintainer.
> -Daniel
>
>
>> +
>> +static const struct drm_connector_helper_funcs drm_panel_connector_hfuncs =
>> {
>> + .get_modes = drm_panel_connector_get_modes,
>> + .best_encoder = drm_panel_connector_best_encoder,
>> +};
>> +
>> +static enum drm_connector_status
>> +drm_panel_connector_detect(struct drm_connector *connector, bool force)
>> +{
>> + if (drm_device_is_unplugged(connector->dev))
>> + return connector_status_disconnected;
>> +
>> + return connector->status;
>> +}
>> +
>> +static void drm_panel_connector_destroy(struct drm_connector *connector)
>> +{
>> + struct drm_panel_connector *panel_connector;
>> +
>> + panel_connector = to_drm_panel_connector(connector);
>> + drm_panel_detach(panel_connector->panel);
>> + drm_panel_remove(panel_connector->panel);
>> + drm_connector_unregister(connector);
>> + drm_connector_cleanup(connector);
>> + kfree(panel_connector);
>> +}
>> +
>> +static const struct drm_connector_funcs drm_panel_connector_funcs = {
>> + .dpms = drm_atomic_helper_connector_dpms,
>> + .reset = drm_atomic_helper_connector_reset,
>> + .detect = drm_panel_connector_detect,
>> + .fill_modes = drm_helper_probe_single_connector_modes,
>> + .destroy = drm_panel_connector_destroy,
>> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> +};
>> +
>> +/**
>> + * drm_panel_connector_create - Create simple connector for panel
>> + * @dev: DRM device
>> + * @panel: DRM panel
>> + * @connector_type: user visible type of the connector
>> + *
>> + * Creates a simple connector for a panel.
>> + * The panel needs to provide a get_modes() function.
>> + *
>> + * Returns:
>> + * Pointer to new connector or ERR_PTR on failure.
>> + */
>> +struct drm_connector *drm_panel_connector_create(struct drm_device *dev,
>> + struct drm_panel *panel,
>> + int connector_type)
>> +{
>> + struct drm_panel_connector *panel_connector;
>> + struct drm_connector *connector;
>> + int ret;
>> +
>> + panel_connector = kzalloc(sizeof(*panel_connector), GFP_KERNEL);
>> + if (!panel_connector)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + panel_connector->panel = panel;
>> + connector = &panel_connector->base;
>> + drm_connector_helper_add(connector, &drm_panel_connector_hfuncs);
>> + ret = drm_connector_init(dev, connector, &drm_panel_connector_funcs,
>> + connector_type);
>> + if (ret) {
>> + kfree(panel_connector);
>> + return ERR_PTR(ret);
>> + }
>> +
>> + connector->status = connector_status_connected;
>> + drm_panel_init(panel);
>> + drm_panel_add(panel);
>> + drm_panel_attach(panel, connector);
>> +
>> + return connector;
>> +}
>> +EXPORT_SYMBOL(drm_panel_connector_create);
>> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
>> index 1500ab9..87264a3 100644
>> --- a/drivers/gpu/drm/panel/Kconfig
>> +++ b/drivers/gpu/drm/panel/Kconfig
>> @@ -4,6 +4,13 @@ config DRM_PANEL
>> help
>> Panel registration and lookup framework.
>>
>> +config DRM_PANEL_HELPER
>> + bool
>> + depends on DRM
>> + select DRM_PANEL
>> + help
>> + Panel helper.
>> +
>> menu "Display Panels"
>> depends on DRM && DRM_PANEL
>>
>> diff --git a/include/drm/drm_panel_helper.h b/include/drm/drm_panel_helper.h
>> new file mode 100644
>> index 0000000..c3355e3
>> --- /dev/null
>> +++ b/include/drm/drm_panel_helper.h
>> @@ -0,0 +1,20 @@
>> +/*
>> + * Copyright (C) 2016 Noralf Trønnes
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#ifndef __DRM_PANEL_HELPER_H__
>> +#define __DRM_PANEL_HELPER_H__
>> +
>> +struct drm_device;
>> +struct drm_panel;
>> +
>> +struct drm_connector *drm_panel_connector_create(struct drm_device *dev,
>> + struct drm_panel *panel,
>> + int connector_type);
>> +
>> +#endif
>> --
>> 2.2.2
>>
