All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Jacopo Mondi <jacopo+renesas@jmondi.org>
Cc: kieran.bingham+renesas@ideasonboard.com, geert@linux-m68k.org,
	horms@verge.net.au, uli@fpond.eu, airlied@linux.ie,
	daniel@ffwll.ch, koji.matsuoka.xm@renesas.com, muroya@ksk.co.jp,
	VenkataRajesh.Kalakodima@in.bosch.com,
	Harsha.ManjulaMallikarjun@in.bosch.com,
	linux-renesas-soc@vger.kernel.org,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 10/14] drm: rcar-du: kms: Collect CMM instances
Date: Tue, 27 Aug 2019 02:51:27 +0300	[thread overview]
Message-ID: <20190826235127.GA5274@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20190825135154.11488-11-jacopo+renesas@jmondi.org>

Hi Jacopo,

Thank you for the patch.

On Sun, Aug 25, 2019 at 03:51:50PM +0200, Jacopo Mondi wrote:
> Implement device tree parsing to collect the available CMM instances
> described by the 'cmms' property. Associate CMMs with CRTCs and store a
> mask of active CMMs in the DU group for later enablement.
> 
> Enforce the suspend/resume ordering of DU and CMM by creating a
> stateless device link between the two to make sure the CMM supplier
> device suspends after and resumes before the DU consumer.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c  |  6 +++
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.h  |  2 +
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h   |  3 ++
>  drivers/gpu/drm/rcar-du/rcar_du_group.h |  2 +
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c   | 63 +++++++++++++++++++++++++
>  5 files changed, 76 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 2da46e3dc4ae..23f1d6cc1719 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -1194,6 +1194,12 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int swindex,
>  	if (ret < 0)
>  		return ret;
>  
> +	/* CMM might be disabled for this CRTC. */
> +	if (rcdu->cmms[swindex]) {
> +		rcrtc->cmm = rcdu->cmms[swindex];
> +		rgrp->cmms_mask |= BIT(hwindex % 2);
> +	}
> +
>  	drm_crtc_helper_add(crtc, &crtc_helper_funcs);
>  
>  	/* Start with vertical blanking interrupt reporting disabled. */
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> index 3b7fc668996f..5f2940c42225 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> @@ -39,6 +39,7 @@ struct rcar_du_vsp;
>   * @vblank_wait: wait queue used to signal vertical blanking
>   * @vblank_count: number of vertical blanking interrupts to wait for
>   * @group: CRTC group this CRTC belongs to
> + * @cmm: CMM associated with this CRTC
>   * @vsp: VSP feeding video to this CRTC
>   * @vsp_pipe: index of the VSP pipeline feeding video to this CRTC
>   * @writeback: the writeback connector
> @@ -64,6 +65,7 @@ struct rcar_du_crtc {
>  	unsigned int vblank_count;
>  
>  	struct rcar_du_group *group;
> +	struct platform_device *cmm;
>  	struct rcar_du_vsp *vsp;
>  	unsigned int vsp_pipe;
>  
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> index a00dccc447aa..300ec60ba31b 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> @@ -13,6 +13,7 @@
>  #include <linux/kernel.h>
>  #include <linux/wait.h>
>  
> +#include "rcar_cmm.h"
>  #include "rcar_du_crtc.h"
>  #include "rcar_du_group.h"
>  #include "rcar_du_vsp.h"
> @@ -70,6 +71,7 @@ struct rcar_du_device_info {
>  
>  #define RCAR_DU_MAX_CRTCS		4
>  #define RCAR_DU_MAX_GROUPS		DIV_ROUND_UP(RCAR_DU_MAX_CRTCS, 2)
> +#define RCAR_DU_MAX_CMMS		4

Maybe alphabetically sorted ?

>  #define RCAR_DU_MAX_VSPS		4
>  
>  struct rcar_du_device {
> @@ -86,6 +88,7 @@ struct rcar_du_device {
>  	struct rcar_du_encoder *encoders[RCAR_DU_OUTPUT_MAX];
>  
>  	struct rcar_du_group groups[RCAR_DU_MAX_GROUPS];
> +	struct platform_device *cmms[RCAR_DU_MAX_CMMS];

As there's one CMM per CRTC, should we use RCAR_DU_MAX_CRTCS here ? It's
not very useful to have two different macros that are required to have
the same value :-)

>  	struct rcar_du_vsp vsps[RCAR_DU_MAX_VSPS];
>  
>  	struct {
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.h b/drivers/gpu/drm/rcar-du/rcar_du_group.h
> index 87950c1f6a52..e9906609c635 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.h
> @@ -22,6 +22,7 @@ struct rcar_du_device;
>   * @mmio_offset: registers offset in the device memory map
>   * @index: group index
>   * @channels_mask: bitmask of populated DU channels in this group
> + * @cmms_mask: bitmask of available CMMs in this group
>   * @num_crtcs: number of CRTCs in this group (1 or 2)
>   * @use_count: number of users of the group (rcar_du_group_(get|put))
>   * @used_crtcs: number of CRTCs currently in use
> @@ -37,6 +38,7 @@ struct rcar_du_group {
>  	unsigned int index;
>  
>  	unsigned int channels_mask;
> +	unsigned int cmms_mask;
>  	unsigned int num_crtcs;
>  	unsigned int use_count;
>  	unsigned int used_crtcs;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> index 2dc9caee8767..61ca1d3c379a 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -17,7 +17,9 @@
>  #include <drm/drm_probe_helper.h>
>  #include <drm/drm_vblank.h>
>  
> +#include <linux/device.h>
>  #include <linux/of_graph.h>
> +#include <linux/of_platform.h>
>  #include <linux/wait.h>
>  
>  #include "rcar_du_crtc.h"
> @@ -614,6 +616,62 @@ static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
>  	return ret;
>  }
>  
> +static int rcar_du_cmm_init(struct rcar_du_device *rcdu)
> +{
> +	const struct device_node *np = rcdu->dev->of_node;
> +	unsigned int i;
> +	int cells;
> +
> +	cells = of_property_count_u32_elems(np, "cmms");
> +	if (cells == -EINVAL)
> +		return 0;
> +
> +	if (cells > RCAR_DU_MAX_CMMS || cells > rcdu->num_crtcs) {

And if we remove RCAR_DU_MAX_CMMS as proposed above, I would just remove
the first part of the condition, as rcdu->num_crtcs is guaranteed to be
<= RCAR_DU_MAX_CRTCS.

With this addressed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +		dev_err(rcdu->dev,
> +			"Invalid number of entries in 'cmms' property\n");
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i < cells; ++i) {
> +		struct platform_device *pdev;
> +		struct device_link *link;
> +		struct device_node *cmm;
> +
> +		cmm = of_parse_phandle(np, "cmms", i);
> +		if (IS_ERR(cmm)) {
> +			dev_err(rcdu->dev, "Failed to parse 'cmms' property\n");
> +			return PTR_ERR(cmm);
> +		}
> +
> +		if (!of_device_is_available(cmm)) {
> +			/* It's fine to have a phandle to a non-enabled CMM. */
> +			of_node_put(cmm);
> +			continue;
> +		}
> +
> +		pdev = of_find_device_by_node(cmm);
> +		if (IS_ERR(pdev)) {
> +			dev_err(rcdu->dev, "No device found for CMM%u\n", i);
> +			of_node_put(cmm);
> +			return PTR_ERR(pdev);
> +		}
> +
> +		of_node_put(cmm);
> +
> +		link = device_link_add(rcdu->dev, &pdev->dev,
> +				       DL_FLAG_STATELESS);
> +		if (!link) {
> +			dev_err(rcdu->dev,
> +				"Failed to create device link to CMM%u\n", i);
> +			return -EINVAL;
> +		}
> +
> +		rcdu->cmms[i] = pdev;
> +	}
> +
> +	return 0;
> +}
> +
>  int rcar_du_modeset_init(struct rcar_du_device *rcdu)
>  {
>  	static const unsigned int mmio_offsets[] = {
> @@ -704,6 +762,11 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
>  			return ret;
>  	}
>  
> +	/* Initialize the Color Management Modules. */
> +	ret = rcar_du_cmm_init(rcdu);
> +	if (ret)
> +		return ret;
> +
>  	/* Create the CRTCs. */
>  	for (swindex = 0, hwindex = 0; swindex < rcdu->num_crtcs; ++hwindex) {
>  		struct rcar_du_group *rgrp;

-- 
Regards,

Laurent Pinchart

  reply	other threads:[~2019-08-26 23:51 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-25 13:51 [PATCH v3 00/14] drm: rcar-du: Add Color Management Module (CMM) Jacopo Mondi
2019-08-25 13:51 ` [PATCH v3 01/14] dt-bindings: display: renesas,cmm: Add R-Car CMM documentation Jacopo Mondi
2019-08-26  7:34   ` Geert Uytterhoeven
2019-08-26  7:34     ` Geert Uytterhoeven
2019-08-26  7:59     ` Jacopo Mondi
2019-08-26  7:59       ` Jacopo Mondi
2019-08-26  8:38       ` Geert Uytterhoeven
2019-08-26  8:38         ` Geert Uytterhoeven
2019-08-26 10:15       ` Laurent Pinchart
2019-08-26 10:15         ` Laurent Pinchart
2019-08-30 18:01         ` Jacopo Mondi
2019-08-30 18:01           ` Jacopo Mondi
2019-09-05 11:50           ` Laurent Pinchart
2019-09-05 11:50             ` Laurent Pinchart
2019-09-05 12:05             ` Geert Uytterhoeven
2019-09-05 12:05               ` Geert Uytterhoeven
2019-09-05 12:20               ` Laurent Pinchart
2019-09-05 12:20                 ` Laurent Pinchart
2019-09-05 13:28                 ` Jacopo Mondi
2019-09-05 13:28                   ` Jacopo Mondi
2019-08-25 13:51 ` [PATCH v3 02/14] dt-bindings: display, renesas,du: Document cmms property Jacopo Mondi
2019-08-27 20:29   ` Rob Herring
2019-08-28  7:32     ` Geert Uytterhoeven
2019-08-28  7:32       ` [PATCH v3 02/14] dt-bindings: display, renesas, du: " Geert Uytterhoeven
2019-08-28  8:28       ` [PATCH v3 02/14] dt-bindings: display, renesas,du: " Laurent Pinchart
2019-08-28  8:28         ` Laurent Pinchart
2019-08-25 13:51 ` [PATCH v3 03/14] arm64: dts: renesas: r8a7796: Add CMM units Jacopo Mondi
2019-08-26  7:28   ` Geert Uytterhoeven
2019-08-26  8:00     ` Jacopo Mondi
2019-08-26 22:43   ` Laurent Pinchart
2019-08-27  9:55     ` Jacopo Mondi
2019-08-27 10:12       ` Geert Uytterhoeven
2019-08-25 13:51 ` [PATCH v3 04/14] arm64: dts: renesas: r8a7795: " Jacopo Mondi
2019-08-26 22:45   ` Laurent Pinchart
2019-08-25 13:51 ` [PATCH v3 05/14] arm64: dts: renesas: r8a77965: " Jacopo Mondi
2019-08-26 22:45   ` Laurent Pinchart
2019-08-25 13:51 ` [PATCH v3 06/14] arm64: dts: renesas: r8a77990: " Jacopo Mondi
2019-08-26 22:47   ` Laurent Pinchart
2019-08-25 13:51 ` [PATCH v3 07/14] arm64: dts: renesas: r8a77995: " Jacopo Mondi
2019-08-26 22:47   ` Laurent Pinchart
2019-08-25 13:51 ` [PATCH v3 08/14] drm: rcar-du: Add support for CMM Jacopo Mondi
2019-08-26  7:31   ` Geert Uytterhoeven
2019-08-26  8:02     ` Jacopo Mondi
2019-08-27  0:24   ` Laurent Pinchart
2019-08-27  0:24     ` Laurent Pinchart
2019-08-27 14:56     ` Jacopo Mondi
2019-08-27 15:48       ` Jacopo Mondi
2019-08-27 16:34       ` Laurent Pinchart
2019-08-27 16:34         ` Laurent Pinchart
2019-09-05  9:57         ` Jacopo Mondi
2019-09-05 11:17           ` Laurent Pinchart
2019-09-05 13:14             ` Jacopo Mondi
2019-09-05 13:39               ` Laurent Pinchart
2019-08-25 13:51 ` [PATCH v3 09/14] drm: rcar-du: Claim CMM support for Gen3 SoCs Jacopo Mondi
2019-08-25 13:51 ` [PATCH v3 10/14] drm: rcar-du: kms: Collect CMM instances Jacopo Mondi
2019-08-26 23:51   ` Laurent Pinchart [this message]
2019-08-25 13:51 ` [PATCH v3 11/14] drm: rcar-du: crtc: Enable and disable CMMs Jacopo Mondi
2019-08-25 13:51 ` [PATCH v3 12/14] drm: rcar-du: crtc: Register GAMMA_LUT properties Jacopo Mondi
2019-08-25 13:51 ` [PATCH v3 13/14] drm: rcar-du: kms: Update CMM in atomic commit tail Jacopo Mondi
2019-08-27  0:00   ` Laurent Pinchart
2019-08-27  0:19     ` Laurent Pinchart
2019-08-27 14:44       ` Jacopo Mondi
2019-08-27 16:38         ` Laurent Pinchart
2019-08-27 16:38           ` Laurent Pinchart
2019-08-25 13:51 ` [PATCH v3 14/14] drm: rcar-du: Force CMM enablement when resuming Jacopo Mondi
2019-08-27  0:05   ` Laurent Pinchart
2019-09-05 10:58     ` Jacopo Mondi
2019-09-05 10:58       ` Jacopo Mondi
2019-09-05 11:25       ` Laurent Pinchart

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=20190826235127.GA5274@pendragon.ideasonboard.com \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=Harsha.ManjulaMallikarjun@in.bosch.com \
    --cc=VenkataRajesh.Kalakodima@in.bosch.com \
    --cc=airlied@linux.ie \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=geert@linux-m68k.org \
    --cc=horms@verge.net.au \
    --cc=jacopo+renesas@jmondi.org \
    --cc=kieran.bingham+renesas@ideasonboard.com \
    --cc=koji.matsuoka.xm@renesas.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=muroya@ksk.co.jp \
    --cc=uli@fpond.eu \
    /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: link
Be 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.