Hi Benoit,
On 02/03/18 15:48, Benoit Parrot wrote:
> Virtual planes are used to extend display size capability for display
> larger than 2048 pixels by splitting the frame buffer equally between
> two physical planes.
>
> Here we are adding DT support to parse 'plane' child nodes which
> describe how logical planes are mapped to physical plane(s) and
> which crtc they are available on.
>
> Signed-off-by: Benoit Parrot <bparrot@xxxxxx>
> ---
> drivers/gpu/drm/omapdrm/dss/dispc.c | 110 ++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/omapdrm/dss/omapdss.h | 11 ++++
> 2 files changed, 121 insertions(+)
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
> index 624dee22f46b..559b70d9762d 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -4334,6 +4334,115 @@ static u32 dispc_get_memory_bandwidth_limit(void)
> return limit;
> }
>
> +static struct device_node *dispc_of_get_plane_by_id(struct device_node *node,
> + u32 id)
> +{
> + struct device_node *plane;
> +
> + for_each_child_of_node(node, plane) {
> + u32 plane_id = 0;
> +
> + if (of_node_cmp(plane->name, "plane") != 0)
> + continue;
> + of_property_read_u32(plane, "reg", &plane_id);
> + if (id == plane_id)
> + break;
I think the flow is more readable, if here you "return plane", and at
the end of the func "return NULL".
> + }
> +
> + return plane;
> +}
> +
> +static int dispc_parse_dt_plane_data(struct dispc_plane_mappings *plane)
You could rename the parameter to "map", for example. Using 'plane'
there is quite confusing.
> +{
> + struct platform_device *pdev = dispc.pdev;
> + struct device_node *np = pdev->dev.of_node;
> + struct device_node *ep;
> + struct property *prop;
> + const __be32 *cur;
> + u32 v;
> + u32 num_ovls = dispc_get_num_ovls();
> + unsigned long int hw_plane_mask = (1 << num_ovls) - 1;
u32?
> + u32 num_planes;
> + int i, index;
> +
> + if (!np)
> + return 0;
> +
> + for (i = 0; i < num_ovls; i++) {
> + ep = dispc_of_get_plane_by_id(np, i);
> + if (!ep)
> + break;
> + if (!of_property_read_bool(ep, "hw-planes")) {
> + dev_err(&pdev->dev,
> + "malformed plane node: hw-planes missing.\n");
> + return -EINVAL;
> + }
> +
> + index = 0;
> + of_property_for_each_u32(ep, "hw-planes", prop, cur, v) {
> + if (v >= num_ovls) {
> + dev_err(&pdev->dev,
> + "hw-planes property: '%d' out-of-range.\n",
> + v);
> + return -EINVAL;
> + }
> + if (!(hw_plane_mask & BIT_MASK(v))) {
> + dev_err(&pdev->dev,
> + "hw-planes property: '%d' used more than once.\n",
> + v);
> + return -EINVAL;
> + }
> + clear_bit(v, &hw_plane_mask);
Why do you use hw_plane_mask this way? It feels inverse to me, usually
one sets a bit when something is there. And e.g. crtc_mask here is used
the other way.
> +
> + if (index == 0) {
> + plane->plane[i].main_id = v;
> + } else if (index == 1) {
> + plane->plane[i].aux_id = v;
> + plane->plane[i].is_virtual = true;
> + } else if (index > 1) {
> + dev_err(&pdev->dev,
> + "hw-planes property: more than 2 values specified.\n");
> + return -EINVAL;
> + }
> + index++;
> + }
> +
> + of_property_for_each_u32(ep, "hw-crtcs", prop, cur, v) {
> + if (v >= num_ovls) {
This should check against num_ovl_mgrs.
> + dev_err(&pdev->dev,
> + "hw-crtcs property: '%d' out-of-range.\n",
> + v);
> + return -EINVAL;
> + }
> + plane->plane[i].crtc_mask |= 1 << v;
> + }
> + }
> +
> + num_planes = i;
> +
> + if (num_planes) {
> + dev_dbg(&pdev->dev, "Plane definitions found from DT:");
> + for (i = 0; i < num_planes; i++) {
> + if (plane->plane[i].is_virtual) {
> + dev_dbg(&pdev->dev,
> + "plane%d: virtual hw-planes: %d, %d crtc_mask: 0x%04x",
> + i, plane->plane[i].main_id,
> + plane->plane[i].aux_id,
> + plane->plane[i].crtc_mask);
> + } else {
> + dev_dbg(&pdev->dev,
> + "plane%d: hw-planes: %d crtc_mask: 0x%04x",
> + i, plane->plane[i].main_id,
> + plane->plane[i].crtc_mask);
> + }
> + }
> + }
> +
> + plane->num_planes = num_planes;
> +
> + return 0;
> +}
> +
> /*
> * Workaround for errata i734 in DSS dispc
> * - LCD1 Gamma Correction Is Not Working When GFX Pipe Is Disabled
> @@ -4525,6 +4634,7 @@ static const struct dispc_ops dispc_ops = {
> .ovl_enable = dispc_ovl_enable,
> .ovl_setup = dispc_ovl_setup,
> .ovl_get_color_modes = dispc_ovl_get_color_modes,
> + .get_plane_mapping = dispc_parse_dt_plane_data,
> };
>
> /* DISPC HW IP initialisation */
> diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> index f8f83e826a56..b8c9b30bf5ed 100644
> --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> @@ -637,6 +637,16 @@ void omapdss_set_is_initialized(bool set);
> struct device_node *dss_of_port_get_parent_device(struct device_node *port);
> u32 dss_of_port_get_port_number(struct device_node *port);
>
> +struct dispc_plane_mappings {
> + struct {
> + u32 main_id;
> + u32 aux_id;
> + u32 crtc_mask;
> + bool is_virtual;
is_virtual has a tab whereas the others don't.
Tomi
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
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
