Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v5 05/13] soc: mediatek: cmdq: return send msg error code
From: Matthias Brugger @ 2020-05-16 17:56 UTC (permalink / raw)
  To: Dennis YC Hsieh, Rob Herring, Mark Rutland, Jassi Brar,
	Philipp Zabel, David Airlie, Daniel Vetter
  Cc: linux-kernel, linux-mediatek, devicetree, wsd_upstream, dri-devel,
	Bibby Hsieh, CK Hu, Houlong Wei, linux-arm-kernel, HS Liao
In-Reply-To: <1583664775-19382-6-git-send-email-dennis-yc.hsieh@mediatek.com>



On 08/03/2020 11:52, Dennis YC Hsieh wrote:
> Return error code to client if send message fail,
> so that client has chance to error handling.
> 
> Signed-off-by: Dennis YC Hsieh <dennis-yc.hsieh@mediatek.com>
> Fixes: 576f1b4bc802 ("soc: mediatek: Add Mediatek CMDQ helper")
> Reviewed-by: CK Hu <ck.hu@mediatek.com>

Queued for v5.7-fixes

> ---
>  drivers/soc/mediatek/mtk-cmdq-helper.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index 2e1bc513569b..98f23ba3ba47 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> @@ -351,7 +351,9 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt, cmdq_async_flush_cb cb,
>  		spin_unlock_irqrestore(&client->lock, flags);
>  	}
>  
> -	mbox_send_message(client->chan, pkt);
> +	err = mbox_send_message(client->chan, pkt);
> +	if (err < 0)
> +		return err;
>  	/* We can send next packet immediately, so just call txdone. */
>  	mbox_client_txdone(client->chan, 0);
>  
> 

^ permalink raw reply

* Re: [PATCH v5 06/13] soc: mediatek: cmdq: add assign function
From: Matthias Brugger @ 2020-05-16 17:59 UTC (permalink / raw)
  To: Dennis YC Hsieh, Rob Herring, Mark Rutland, Jassi Brar,
	Philipp Zabel, David Airlie, Daniel Vetter
  Cc: linux-kernel, linux-mediatek, devicetree, wsd_upstream, dri-devel,
	Bibby Hsieh, CK Hu, Houlong Wei, linux-arm-kernel, HS Liao
In-Reply-To: <1583664775-19382-7-git-send-email-dennis-yc.hsieh@mediatek.com>



On 08/03/2020 11:52, Dennis YC Hsieh wrote:
> Add assign function in cmdq helper which assign constant value into
> internal register by index.
> 
> Signed-off-by: Dennis YC Hsieh <dennis-yc.hsieh@mediatek.com>
> Reviewed-by: CK Hu <ck.hu@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-cmdq-helper.c   | 24 +++++++++++++++++++++++-
>  include/linux/mailbox/mtk-cmdq-mailbox.h |  1 +
>  include/linux/soc/mediatek/mtk-cmdq.h    | 14 ++++++++++++++
>  3 files changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index 98f23ba3ba47..33153d17c9d9 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> @@ -12,6 +12,7 @@
>  #define CMDQ_WRITE_ENABLE_MASK	BIT(0)
>  #define CMDQ_POLL_ENABLE_MASK	BIT(0)
>  #define CMDQ_EOC_IRQ_EN		BIT(0)
> +#define CMDQ_REG_TYPE		1
>  
>  struct cmdq_instruction {
>  	union {
> @@ -21,8 +22,17 @@ struct cmdq_instruction {
>  	union {
>  		u16 offset;
>  		u16 event;
> +		u16 reg_dst;
> +	};
> +	union {
> +		u8 subsys;
> +		struct {
> +			u8 sop:5;
> +			u8 arg_c_t:1;
> +			u8 arg_b_t:1;
> +			u8 dst_t:1;
> +		};

This union seems without context in this patch. Please drop.

Regards,
Matthias

>  	};
> -	u8 subsys;
>  	u8 op;
>  };
>  
> @@ -277,6 +287,18 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
>  }
>  EXPORT_SYMBOL(cmdq_pkt_poll_mask);
>  
> +int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value)
> +{
> +	struct cmdq_instruction inst = { {0} };
> +
> +	inst.op = CMDQ_CODE_LOGIC;
> +	inst.dst_t = CMDQ_REG_TYPE;
> +	inst.reg_dst = reg_idx;
> +	inst.value = value;
> +	return cmdq_pkt_append_command(pkt, inst);
> +}
> +EXPORT_SYMBOL(cmdq_pkt_assign);
> +
>  static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
>  {
>  	struct cmdq_instruction inst = { {0} };
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> index dfe5b2eb85cc..121c3bb6d3de 100644
> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> @@ -59,6 +59,7 @@ enum cmdq_code {
>  	CMDQ_CODE_JUMP = 0x10,
>  	CMDQ_CODE_WFE = 0x20,
>  	CMDQ_CODE_EOC = 0x40,
> +	CMDQ_CODE_LOGIC = 0xa0,
>  };
>  
>  enum cmdq_cb_status {
> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> index a74c1d5acdf3..83340211e1d3 100644
> --- a/include/linux/soc/mediatek/mtk-cmdq.h
> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> @@ -152,6 +152,20 @@ int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys,
>   */
>  int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
>  		       u16 offset, u32 value, u32 mask);
> +
> +/**
> + * cmdq_pkt_assign() - Append logic assign command to the CMDQ packet, ask GCE
> + *		       to execute an instruction that set a constant value into
> + *		       internal register and use as value, mask or address in
> + *		       read/write instruction.
> + * @pkt:	the CMDQ packet
> + * @reg_idx:	the CMDQ internal register ID
> + * @value:	the specified value
> + *
> + * Return: 0 for success; else the error code is returned
> + */
> +int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value);
> +
>  /**
>   * cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ
>   *                          packet and call back at the end of done packet
> 

^ permalink raw reply

* Re: [PATCH v2 2/7] ASoC: qcom: Add common array to initialize soc based core clocks
From: Ajit Pandey @ 2020-05-16 18:06 UTC (permalink / raw)
  To: Mark Brown
  Cc: plai, bgoswami, srinivas.kandagatla, alsa-devel, devicetree,
	linux-kernel

On 5/14/2020 10:15 PM, Mark Brown wrote:
> On Thu, May 14, 2020 at 10:08:14PM +0530, Ajit Pandey wrote:
>> AHB-I/X bus clocks inside LPASS coreis not a mandatory clocks for
>> SC7180 soc lpass variants, hence make it an optional one.
> Do we need to improve the checks in the rest of the code so that if the
> clock is missing on a SoC where it is actually mandatory we don't end up
> trying to start without it?  Perhaps it's more trouble than it's worth
> though.
> Agreed.. adding a case check with soc id or name definately make sense 
> to avoid issue with soc where it's  a mandatory one, I'll also look if 
> this "ahbix-clk" can also be moved with other soc-based clocks array ? 
> and can be enabled in soc specific driver.

^ permalink raw reply

* Re: [PATCH v2 5/7] include: dt-bindings: sound: Add sc7180-lpass bindings header
From: Ajit Pandey @ 2020-05-16 18:14 UTC (permalink / raw)
  To: Mark Brown
  Cc: plai, bgoswami, srinivas.kandagatla, alsa-devel, devicetree,
	linux-kernel

On 5/14/2020 10:18 PM, Mark Brown wrote:
> On Thu, May 14, 2020 at 10:08:16PM +0530, Ajit Pandey wrote:
>
>> +#define MI2S_PRIMARY	0
>> +#define MI2S_SECONDARY	1
>> +
>> +#define LPASS_MCLK0	0
> These look like they could use namespacing.  Is primary/secondary
> perhaps something that could be a boolean property?

Primary/secondary is a dai id and can be from 0-4 (SC7180 soc driver 
support only 0-1 at present). Such id is mainly used in dtsi and machine 
driver to do dai id based configuration for a soc hence we placed them 
in dt headers file for now.

^ permalink raw reply

* Re: [PATCH v5 07/13] soc: mediatek: cmdq: add write_s function
From: Matthias Brugger @ 2020-05-16 18:14 UTC (permalink / raw)
  To: Dennis YC Hsieh, Rob Herring, Mark Rutland, Jassi Brar,
	Philipp Zabel, David Airlie, Daniel Vetter
  Cc: linux-kernel, linux-mediatek, devicetree, wsd_upstream, dri-devel,
	Bibby Hsieh, CK Hu, Houlong Wei, linux-arm-kernel, HS Liao
In-Reply-To: <1583664775-19382-8-git-send-email-dennis-yc.hsieh@mediatek.com>



On 08/03/2020 11:52, Dennis YC Hsieh wrote:
> add write_s function in cmdq helper functions which
> writes value contains in internal register to address
> with large dma access support.
> 
> Signed-off-by: Dennis YC Hsieh <dennis-yc.hsieh@mediatek.com>
> Reviewed-by: CK Hu <ck.hu@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-cmdq-helper.c   | 34 +++++++++++++++++++++++-
>  include/linux/mailbox/mtk-cmdq-mailbox.h |  2 ++
>  include/linux/soc/mediatek/mtk-cmdq.h    | 20 ++++++++++++++
>  3 files changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index 33153d17c9d9..90f1ff2b4b00 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> @@ -18,6 +18,10 @@ struct cmdq_instruction {
>  	union {
>  		u32 value;
>  		u32 mask;
> +		struct {
> +			u16 arg_c;
> +			u16 src_reg;
> +		};
>  	};
>  	union {
>  		u16 offset;
> @@ -29,7 +33,7 @@ struct cmdq_instruction {
>  		struct {
>  			u8 sop:5;
>  			u8 arg_c_t:1;
> -			u8 arg_b_t:1;
> +			u8 src_t:1;

fixing patch 6/13 please. seems the struct should be added in this patch.

>  			u8 dst_t:1;
>  		};
>  	};
> @@ -222,6 +226,34 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
>  }
>  EXPORT_SYMBOL(cmdq_pkt_write_mask);
>  
> +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
> +		     u16 addr_low, u16 src_reg_idx, u32 mask)
> +{
> +	struct cmdq_instruction inst = { {0} };
> +	int err;
> +
> +	if (mask != U32_MAX) {
> +		inst.op = CMDQ_CODE_MASK;
> +		inst.mask = ~mask;
> +		err = cmdq_pkt_append_command(pkt, inst);
> +		if (err < 0)
> +			return err;
> +
> +		inst.mask = 0;
> +		inst.op = CMDQ_CODE_WRITE_S_MASK;
> +	} else {
> +		inst.op = CMDQ_CODE_WRITE_S;
> +	}
> +
> +	inst.src_t = CMDQ_REG_TYPE;

Not defined.
Please make sure that every patch compiles on it's own and does not add a
regression. This is very helpful if we have to bisect the kernel in the future.

> +	inst.sop = high_addr_reg_idx;
> +	inst.offset = addr_low;
> +	inst.src_reg = src_reg_idx;
> +
> +	return cmdq_pkt_append_command(pkt, inst);
> +}
> +EXPORT_SYMBOL(cmdq_pkt_write_s);
> +
>  int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
>  {
>  	struct cmdq_instruction inst = { {0} };
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> index 121c3bb6d3de..8ef87e1bd03b 100644
> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> @@ -59,6 +59,8 @@ enum cmdq_code {
>  	CMDQ_CODE_JUMP = 0x10,
>  	CMDQ_CODE_WFE = 0x20,
>  	CMDQ_CODE_EOC = 0x40,
> +	CMDQ_CODE_WRITE_S = 0x90,
> +	CMDQ_CODE_WRITE_S_MASK = 0x91,
>  	CMDQ_CODE_LOGIC = 0xa0,
>  };
>  
> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> index 83340211e1d3..c72d826d8934 100644
> --- a/include/linux/soc/mediatek/mtk-cmdq.h
> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> @@ -12,6 +12,8 @@
>  #include <linux/timer.h>
>  
>  #define CMDQ_NO_TIMEOUT		0xffffffffu
> +#define CMDQ_ADDR_HIGH(addr)	((u32)(((addr) >> 16) & GENMASK(31, 0)))
> +#define CMDQ_ADDR_LOW(addr)	((u16)(addr) | BIT(1))
>  
>  struct cmdq_pkt;
>  
> @@ -102,6 +104,24 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16 offset, u32 value);
>  int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
>  			u16 offset, u32 value, u32 mask);
>  
> +/**
> + * cmdq_pkt_write_s() - append write_s command to the CMDQ packet
> + * @pkt:	the CMDQ packet
> + * @high_addr_reg_idx:	internal regisger ID which contains high address of pa

s/regisger/register

> + * @addr_low:	low address of pa
> + * @src_reg_idx:	the CMDQ internal register ID which cache source value
> + * @mask:	the specified target address mask, use U32_MAX if no need
> + *
> + * Return: 0 for success; else the error code is returned
> + *
> + * Support write value to physical address without subsys. Use CMDQ_ADDR_HIGH()
> + * to get high addrees and call cmdq_pkt_assign() to assign value into internal

s/addrees/address

> + * reg. Also use CMDQ_ADDR_LOW() to get low address for addr_low parameterwhen

s/parameterwhen/parameter when

> + * call to this function.
> + */
> +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
> +		     u16 addr_low, u16 src_reg_idx, u32 mask);
> +

In general I wonder if we shouldn't provide two functions, one that writes a
mask and on for the else case.

Regards,
Matthias

>  /**
>   * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
>   * @pkt:	the CMDQ packet
> 

^ permalink raw reply

* Re: [PATCH v5 09/13] soc: mediatek: cmdq: add write_s value function
From: Matthias Brugger @ 2020-05-16 18:20 UTC (permalink / raw)
  To: Dennis YC Hsieh, Rob Herring, Mark Rutland, Jassi Brar,
	Philipp Zabel, David Airlie, Daniel Vetter
  Cc: linux-kernel, linux-mediatek, devicetree, wsd_upstream, dri-devel,
	Bibby Hsieh, CK Hu, Houlong Wei, linux-arm-kernel, HS Liao
In-Reply-To: <1583664775-19382-10-git-send-email-dennis-yc.hsieh@mediatek.com>



On 08/03/2020 11:52, Dennis YC Hsieh wrote:
> add write_s function in cmdq helper functions which
> writes a constant value to address with large dma
> access support.
> 
> Signed-off-by: Dennis YC Hsieh <dennis-yc.hsieh@mediatek.com>
> Reviewed-by: CK Hu <ck.hu@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-cmdq-helper.c | 26 ++++++++++++++++++++++++++
>  include/linux/soc/mediatek/mtk-cmdq.h  | 14 ++++++++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index 03c129230cd7..a9ebbabb7439 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> @@ -269,6 +269,32 @@ int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
>  }
>  EXPORT_SYMBOL(cmdq_pkt_write_s);
>  
> +int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
> +			   u16 addr_low, u32 value, u32 mask)
> +{
> +	struct cmdq_instruction inst = { {0} };
> +	int err;
> +
> +	if (mask != U32_MAX) {
> +		inst.op = CMDQ_CODE_MASK;
> +		inst.mask = ~mask;
> +		err = cmdq_pkt_append_command(pkt, inst);
> +		if (err < 0)
> +			return err;
> +
> +		inst.op = CMDQ_CODE_WRITE_S_MASK;
> +	} else {
> +		inst.op = CMDQ_CODE_WRITE_S;
> +	}
> +
> +	inst.sop = high_addr_reg_idx;

Writing u16 value in a 5 bit wide variable?

> +	inst.offset = addr_low;
> +	inst.value = value;
> +
> +	return cmdq_pkt_append_command(pkt, inst);
> +}
> +EXPORT_SYMBOL(cmdq_pkt_write_s_value);
> +
>  int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
>  {
>  	struct cmdq_instruction inst = { {0} };
> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> index 01b4184af310..fec292aac83c 100644
> --- a/include/linux/soc/mediatek/mtk-cmdq.h
> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> @@ -135,6 +135,20 @@ int cmdq_pkt_read_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, u16 addr_low,
>  int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
>  		     u16 addr_low, u16 src_reg_idx, u32 mask);
>  
> +/**
> + * cmdq_pkt_write_s_value() - append write_s command with mask to the CMDQ
> + *			      packet which write value to a physical address
> + * @pkt:	the CMDQ packet
> + * @high_addr_reg_idx:	internal regisger ID which contains high address of pa

register

> + * @addr_low:	low address of pa
> + * @value:	the specified target value
> + * @mask:	the specified target mask
> + *
> + * Return: 0 for success; else the error code is returned
> + */
> +int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
> +			   u16 addr_low, u32 value, u32 mask);
> +
>  /**
>   * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
>   * @pkt:	the CMDQ packet
> 

^ permalink raw reply

* Re: [PATCH v1 5/9] dt-bindings: dmaengine: convert Actions Semi Owl SoCs bindings to yaml
From: Amit Tomer @ 2020-05-16 18:21 UTC (permalink / raw)
  To: André Przywara
  Cc: Andreas Färber, Manivannan Sadhasivam, Rob Herring,
	cristian.ciocaltea, linux-arm-kernel, linux-actions, devicetree
In-Reply-To: <afc0d7f3-d763-b936-988c-d802b86836bc@arm.com>

Hi,

> (The kernel chose to use only one, but that's nothing the binding is
> concerned about).
But there are four different interrupt events corresponds to these four
interrupts.
So, if Kernel chooses to have only one interrupt , how other events
would be notified ?

Thanks
-Amit

^ permalink raw reply

* Re: [PATCH v5 10/13] soc: mediatek: cmdq: export finalize function
From: Matthias Brugger @ 2020-05-16 18:22 UTC (permalink / raw)
  To: Dennis YC Hsieh, Rob Herring, Mark Rutland, Jassi Brar,
	Philipp Zabel, David Airlie, Daniel Vetter
  Cc: linux-kernel, linux-mediatek, devicetree, wsd_upstream, dri-devel,
	Bibby Hsieh, CK Hu, Houlong Wei, linux-arm-kernel, HS Liao
In-Reply-To: <1583664775-19382-11-git-send-email-dennis-yc.hsieh@mediatek.com>



On 08/03/2020 11:52, Dennis YC Hsieh wrote:
> Export finalize function to client which helps append eoc and jump
> command to pkt. Let client decide call finalize or not.
> 
> Signed-off-by: Dennis YC Hsieh <dennis-yc.hsieh@mediatek.com>
> Reviewed-by: CK Hu <ck.hu@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 1 +
>  drivers/soc/mediatek/mtk-cmdq-helper.c  | 7 ++-----
>  include/linux/soc/mediatek/mtk-cmdq.h   | 8 ++++++++
>  3 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 0dfcd1787e65..7daaabc26eb1 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -490,6 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
>  		cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
>  		cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
>  		mtk_crtc_ddp_config(crtc, cmdq_handle);
> +		cmdq_pkt_finalize(cmdq_handle);
>  		cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
>  	}
>  #endif

This should be a independent patch.
Other then that patch looks good.

> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index a9ebbabb7439..59bc1164b411 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> @@ -372,7 +372,7 @@ int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value)
>  }
>  EXPORT_SYMBOL(cmdq_pkt_assign);
>  
> -static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
> +int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
>  {
>  	struct cmdq_instruction inst = { {0} };
>  	int err;
> @@ -392,6 +392,7 @@ static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
>  
>  	return err;
>  }
> +EXPORT_SYMBOL(cmdq_pkt_finalize);
>  
>  static void cmdq_pkt_flush_async_cb(struct cmdq_cb_data data)
>  {
> @@ -426,10 +427,6 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt, cmdq_async_flush_cb cb,
>  	unsigned long flags = 0;
>  	struct cmdq_client *client = (struct cmdq_client *)pkt->cl;
>  
> -	err = cmdq_pkt_finalize(pkt);
> -	if (err < 0)
> -		return err;
> -
>  	pkt->cb.cb = cb;
>  	pkt->cb.data = data;
>  	pkt->async_cb.cb = cmdq_pkt_flush_async_cb;
> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> index fec292aac83c..99e77155f967 100644
> --- a/include/linux/soc/mediatek/mtk-cmdq.h
> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> @@ -213,6 +213,14 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
>   */
>  int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value);
>  
> +/**
> + * cmdq_pkt_finalize() - Append EOC and jump command to pkt.
> + * @pkt:	the CMDQ packet
> + *
> + * Return: 0 for success; else the error code is returned
> + */
> +int cmdq_pkt_finalize(struct cmdq_pkt *pkt);
> +
>  /**
>   * cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ
>   *                          packet and call back at the end of done packet
> 

^ permalink raw reply

* Re: [PATCH v5 12/13] soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api
From: Matthias Brugger @ 2020-05-16 18:30 UTC (permalink / raw)
  To: Dennis YC Hsieh, Rob Herring, Mark Rutland, Jassi Brar,
	Philipp Zabel, David Airlie, Daniel Vetter
  Cc: linux-kernel, linux-mediatek, devicetree, wsd_upstream, dri-devel,
	Bibby Hsieh, CK Hu, Houlong Wei, linux-arm-kernel, HS Liao
In-Reply-To: <1583664775-19382-13-git-send-email-dennis-yc.hsieh@mediatek.com>



On 08/03/2020 11:52, Dennis YC Hsieh wrote:
> Add clear parameter to let client decide if
> event should be clear to 0 after GCE receive it.
> 
> Signed-off-by: Dennis YC Hsieh <dennis-yc.hsieh@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c  | 2 +-
>  drivers/soc/mediatek/mtk-cmdq-helper.c   | 5 +++--
>  include/linux/mailbox/mtk-cmdq-mailbox.h | 3 +--
>  include/linux/soc/mediatek/mtk-cmdq.h    | 5 +++--
>  4 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 7daaabc26eb1..a065b3a412cf 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -488,7 +488,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
>  	if (mtk_crtc->cmdq_client) {
>  		cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE);
>  		cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
> -		cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
> +		cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false);
>  		mtk_crtc_ddp_config(crtc, cmdq_handle);
>  		cmdq_pkt_finalize(cmdq_handle);
>  		cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);

This should be an independent patch

> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index bb5be20fc70a..ec5637d43254 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> @@ -296,15 +296,16 @@ int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
>  }
>  EXPORT_SYMBOL(cmdq_pkt_write_s_value);
>  
> -int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
> +int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear)
>  {
>  	struct cmdq_instruction inst = { {0} };
> +	u32 clear_option = clear ? CMDQ_WFE_UPDATE : 0;
>  
>  	if (event >= CMDQ_MAX_EVENT)
>  		return -EINVAL;
>  
>  	inst.op = CMDQ_CODE_WFE;
> -	inst.value = CMDQ_WFE_OPTION;
> +	inst.value = CMDQ_WFE_OPTION | clear_option;
>  	inst.event = event;
>  
>  	return cmdq_pkt_append_command(pkt, inst);
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> index 3f6bc0dfd5da..42d2a30e6a70 100644
> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> @@ -27,8 +27,7 @@
>   * bit 16-27: update value
>   * bit 31: 1 - update, 0 - no update
>   */
> -#define CMDQ_WFE_OPTION			(CMDQ_WFE_UPDATE | CMDQ_WFE_WAIT | \
> -					CMDQ_WFE_WAIT_VALUE)
> +#define CMDQ_WFE_OPTION			(CMDQ_WFE_WAIT | CMDQ_WFE_WAIT_VALUE)
>  
>  /** cmdq event maximum */
>  #define CMDQ_MAX_EVENT			0x3ff
> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> index 1a6c56f3bec1..d63749440697 100644
> --- a/include/linux/soc/mediatek/mtk-cmdq.h
> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> @@ -152,11 +152,12 @@ int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
>  /**
>   * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
>   * @pkt:	the CMDQ packet
> - * @event:	the desired event type to "wait and CLEAR"
> + * @event:	the desired event type to wait
> + * @clear:	clear event or not after event arrive
>   *
>   * Return: 0 for success; else the error code is returned
>   */
> -int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event);
> +int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear);
>  
>  /**
>   * cmdq_pkt_clear_event() - append clear event command to the CMDQ packet
> 

^ permalink raw reply

* Re: [PATCH v5 13/13] soc: mediatek: cmdq: add set event function
From: Matthias Brugger @ 2020-05-16 18:32 UTC (permalink / raw)
  To: Dennis YC Hsieh, Rob Herring, Mark Rutland, Jassi Brar,
	Philipp Zabel, David Airlie, Daniel Vetter
  Cc: linux-kernel, linux-mediatek, devicetree, wsd_upstream, dri-devel,
	Bibby Hsieh, CK Hu, Houlong Wei, linux-arm-kernel, HS Liao
In-Reply-To: <1583664775-19382-14-git-send-email-dennis-yc.hsieh@mediatek.com>



On 08/03/2020 11:52, Dennis YC Hsieh wrote:
> Add set event function in cmdq helper functions to set specific event.
> 
> Signed-off-by: Dennis YC Hsieh <dennis-yc.hsieh@mediatek.com>
> Reviewed-by: CK Hu <ck.hu@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-cmdq-helper.c   | 15 +++++++++++++++
>  include/linux/mailbox/mtk-cmdq-mailbox.h |  1 +
>  include/linux/soc/mediatek/mtk-cmdq.h    |  9 +++++++++
>  3 files changed, 25 insertions(+)
> 
> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index ec5637d43254..3294c9285994 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> @@ -327,6 +327,21 @@ int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event)
>  }
>  EXPORT_SYMBOL(cmdq_pkt_clear_event);
>  
> +int cmdq_pkt_set_event(struct cmdq_pkt *pkt, u16 event)
> +{
> +	struct cmdq_instruction inst = { {0} };
> +
> +	if (event >= CMDQ_MAX_EVENT)
> +		return -EINVAL;
> +
> +	inst.op = CMDQ_CODE_WFE;
> +	inst.value = CMDQ_WFE_UPDATE | CMDQ_WFE_UPDATE_VALUE;
> +	inst.event = event;
> +
> +	return cmdq_pkt_append_command(pkt, inst);
> +}
> +EXPORT_SYMBOL(cmdq_pkt_set_event);
> +
>  int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys,
>  		  u16 offset, u32 value)
>  {
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> index 42d2a30e6a70..ba2d811183a9 100644
> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> @@ -17,6 +17,7 @@
>  #define CMDQ_JUMP_PASS			CMDQ_INST_SIZE
>  
>  #define CMDQ_WFE_UPDATE			BIT(31)
> +#define CMDQ_WFE_UPDATE_VALUE		BIT(16)
>  #define CMDQ_WFE_WAIT			BIT(15)
>  #define CMDQ_WFE_WAIT_VALUE		0x1
>  
> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> index d63749440697..ca70296ae120 100644
> --- a/include/linux/soc/mediatek/mtk-cmdq.h
> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> @@ -168,6 +168,15 @@ int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear);
>   */
>  int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event);
>  
> +/**
> + * cmdq_pkt_set_event() - append set event command to the CMDQ packet
> + * @pkt:	the CMDQ packet
> + * @event:	the desired event to be set

Can we add the events and their code, so that later on, when a consumer calls
cmdq_pkt_set_event() we don't have any magic values that are hard to understand?

Regards,
Matthias

> + *
> + * Return: 0 for success; else the error code is returned
> + */
> +int cmdq_pkt_set_event(struct cmdq_pkt *pkt, u16 event);
> +
>  /**
>   * cmdq_pkt_poll() - Append polling command to the CMDQ packet, ask GCE to
>   *		     execute an instruction that wait for a specified
> 

^ permalink raw reply

* Re: [Linux-stm32] [PATCH v7 5/6] clocksource: Add Low Power STM32 timers driver
From: Benjamin GAIGNARD @ 2020-05-16 18:51 UTC (permalink / raw)
  To: Daniel Lezcano, Fabrice GASNIER, lee.jones@linaro.org,
	robh+dt@kernel.org, mark.rutland@arm.com,
	mcoquelin.stm32@gmail.com, Alexandre TORGUE, tglx@linutronix.de
  Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Pascal PAILLET-LME, Benjamin Gaignard,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <103c5558-4dc9-63c9-4994-5c8f97646eee@linaro.org>



On 5/15/20 5:25 PM, Daniel Lezcano wrote:
> Hi Benjamin,
>
> On 05/05/2020 09:26, Benjamin GAIGNARD wrote:
>>
>> On 4/20/20 2:16 PM, Benjamin Gaignard wrote:
>>> From: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>>>
>>> Implement clock event driver using low power STM32 timers.
>>> Low power timer counters running even when CPUs are stopped.
>>> It could be used as clock event broadcaster to wake up CPUs but not like
>>> a clocksource because each it rise an interrupt the counter restart from 0.
>>>
>>> Low power timers have a 16 bits counter and a prescaler which allow to
>>> divide the clock per power of 2 to up 128 to target a 32KHz rate.
>> Gentle ping to reviewers on this driver part of the series.
>> The bindings and the MFD have been reviewed so I hope I can progress
>> on the driver part too.
> [ ... ]
>
> sorry for the delay.
>
> How do you want these patches to be merged?
>
> Shall I pick patch 6/7 ?
If Lee agrees I think the best is to get all the patches in mfd tree because
of the dependencies between them.

Benjamin
>
>

^ permalink raw reply

* [PATCH 1/2] dt-bindings: mfd: max8998: Document charger regulator
From: Jonathan Bakker @ 2020-05-16 19:47 UTC (permalink / raw)
  To: lgirdwood, broonie, robh+dt
  Cc: lee.jones, devicetree, linux-kernel, Jonathan Bakker

max8998 has a current regulator for control of its charging
current.  Document it.

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
---
 Documentation/devicetree/bindings/mfd/max8998.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/max8998.txt b/Documentation/devicetree/bindings/mfd/max8998.txt
index 5f2f07c09c90..4ed52184d081 100644
--- a/Documentation/devicetree/bindings/mfd/max8998.txt
+++ b/Documentation/devicetree/bindings/mfd/max8998.txt
@@ -73,6 +73,8 @@ number as described in MAX8998 datasheet.
 	- ESAFEOUT1: (ldo19)
 	- ESAFEOUT2: (ld020)
 
+	- CHARGER: main battery charger current control
+
 Standard regulator bindings are used inside regulator subnodes. Check
   Documentation/devicetree/bindings/regulator/regulator.txt
 for more details.
@@ -113,5 +115,11 @@ Example:
 				regulator-always-on;
 				regulator-boot-on;
 			};
+
+			charger_reg: CHARGER {
+				regulator-name = "CHARGER";
+				regulator-min-microamp = <90000>;
+				regulator-max-microamp = <800000>;
+			};
 		};
 	};
-- 
2.20.1


^ permalink raw reply related

* [PATCH 2/2] regulator: max8998: Add charger regulator
From: Jonathan Bakker @ 2020-05-16 19:47 UTC (permalink / raw)
  To: lgirdwood, broonie, robh+dt
  Cc: lee.jones, devicetree, linux-kernel, Jonathan Bakker
In-Reply-To: <20200516194759.7643-1-xc-racer2@live.ca>

The max8998 has a current regulator for charging control.  The
charger driver in drivers/power/supply/max8998_charger.c has a
comment in it stating that 'charger control is done by a current
regulator "CHARGER"', but this regulator was never added until
now.

The current values have been extracted from a downstream driver
for the SGH-T959V.

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
---
 drivers/regulator/max8998.c | 105 ++++++++++++++++++++++++++++++++++++
 include/linux/mfd/max8998.h |   1 +
 2 files changed, 106 insertions(+)

diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index 60599c3bb845..668ced006417 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -33,6 +33,10 @@ struct max8998_data {
 	unsigned int		buck2_idx;
 };
 
+static const unsigned int charger_current_table[] = {
+	90000, 380000, 475000, 550000, 570000, 600000, 700000, 800000,
+};
+
 static int max8998_get_enable_register(struct regulator_dev *rdev,
 					int *reg, int *shift)
 {
@@ -63,6 +67,10 @@ static int max8998_get_enable_register(struct regulator_dev *rdev,
 		*reg = MAX8998_REG_CHGR2;
 		*shift = 7 - (ldo - MAX8998_ESAFEOUT1);
 		break;
+	case MAX8998_CHARGER:
+		*reg = MAX8998_REG_CHGR2;
+		*shift = 0;
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -88,6 +96,11 @@ static int max8998_ldo_is_enabled(struct regulator_dev *rdev)
 	return val & (1 << shift);
 }
 
+static int max8998_ldo_is_enabled_inverted(struct regulator_dev *rdev)
+{
+	return (!max8998_ldo_is_enabled(rdev));
+}
+
 static int max8998_ldo_enable(struct regulator_dev *rdev)
 {
 	struct max8998_data *max8998 = rdev_get_drvdata(rdev);
@@ -358,6 +371,74 @@ static int max8998_set_voltage_buck_time_sel(struct regulator_dev *rdev,
 	return 0;
 }
 
+int max8998_set_current_limit(struct regulator_dev *rdev,
+			      int min_uA, int max_uA)
+{
+	struct max8998_data *max8998 = rdev_get_drvdata(rdev);
+	struct i2c_client *i2c = max8998->iodev->i2c;
+	unsigned int n_currents = rdev->desc->n_current_limits;
+	int i, sel = -1;
+
+	if (n_currents == 0)
+		return -EINVAL;
+
+	if (rdev->desc->curr_table) {
+		const unsigned int *curr_table = rdev->desc->curr_table;
+		bool ascend = curr_table[n_currents - 1] > curr_table[0];
+
+		/* search for closest to maximum */
+		if (ascend) {
+			for (i = n_currents - 1; i >= 0; i--) {
+				if (min_uA <= curr_table[i] &&
+				    curr_table[i] <= max_uA) {
+					sel = i;
+					break;
+				}
+			}
+		} else {
+			for (i = 0; i < n_currents; i++) {
+				if (min_uA <= curr_table[i] &&
+				    curr_table[i] <= max_uA) {
+					sel = i;
+					break;
+				}
+			}
+		}
+	}
+
+	if (sel < 0)
+		return -EINVAL;
+
+	sel <<= ffs(rdev->desc->csel_mask) - 1;
+
+	return max8998_update_reg(i2c, rdev->desc->csel_reg,
+				  sel, rdev->desc->csel_mask);
+}
+
+int max8998_get_current_limit(struct regulator_dev *rdev)
+{
+	struct max8998_data *max8998 = rdev_get_drvdata(rdev);
+	struct i2c_client *i2c = max8998->iodev->i2c;
+	u8 val;
+	int ret;
+
+	ret = max8998_read_reg(i2c, rdev->desc->csel_reg, &val);
+	if (ret != 0)
+		return ret;
+
+	val &= rdev->desc->csel_mask;
+	val >>= ffs(rdev->desc->csel_mask) - 1;
+
+	if (rdev->desc->curr_table) {
+		if (val >= rdev->desc->n_current_limits)
+			return -EINVAL;
+
+		return rdev->desc->curr_table[val];
+	}
+
+	return -EINVAL;
+}
+
 static const struct regulator_ops max8998_ldo_ops = {
 	.list_voltage		= regulator_list_voltage_linear,
 	.map_voltage		= regulator_map_voltage_linear,
@@ -379,6 +460,15 @@ static const struct regulator_ops max8998_buck_ops = {
 	.set_voltage_time_sel	= max8998_set_voltage_buck_time_sel,
 };
 
+static const struct regulator_ops max8998_charger_ops = {
+	.set_current_limit	= max8998_set_current_limit,
+	.get_current_limit	= max8998_get_current_limit,
+	.is_enabled		= max8998_ldo_is_enabled_inverted,
+	/* Swapped as register is inverted */
+	.enable			= max8998_ldo_disable,
+	.disable		= max8998_ldo_enable,
+};
+
 static const struct regulator_ops max8998_others_ops = {
 	.is_enabled		= max8998_ldo_is_enabled,
 	.enable			= max8998_ldo_enable,
@@ -397,6 +487,19 @@ static const struct regulator_ops max8998_others_ops = {
 		.owner = THIS_MODULE, \
 	}
 
+#define MAX8998_CURRENT_REG(_name, _ops, _table, _reg, _mask) \
+	{ \
+		.name = #_name, \
+		.id = MAX8998_##_name, \
+		.ops = _ops, \
+		.curr_table = _table, \
+		.n_current_limits = ARRAY_SIZE(_table), \
+		.csel_reg = _reg, \
+		.csel_mask = _mask, \
+		.type = REGULATOR_CURRENT, \
+		.owner = THIS_MODULE, \
+	}
+
 #define MAX8998_OTHERS_REG(_name, _id) \
 	{ \
 		.name = #_name, \
@@ -432,6 +535,8 @@ static const struct regulator_desc regulators[] = {
 	MAX8998_OTHERS_REG(ENVICHG, MAX8998_ENVICHG),
 	MAX8998_OTHERS_REG(ESAFEOUT1, MAX8998_ESAFEOUT1),
 	MAX8998_OTHERS_REG(ESAFEOUT2, MAX8998_ESAFEOUT2),
+	MAX8998_CURRENT_REG(CHARGER, &max8998_charger_ops,
+			    charger_current_table, MAX8998_REG_CHGR1, 0x7),
 };
 
 static int max8998_pmic_dt_parse_dvs_gpio(struct max8998_dev *iodev,
diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
index 061af220dcd3..79c020bd0c70 100644
--- a/include/linux/mfd/max8998.h
+++ b/include/linux/mfd/max8998.h
@@ -39,6 +39,7 @@ enum {
 	MAX8998_ENVICHG,
 	MAX8998_ESAFEOUT1,
 	MAX8998_ESAFEOUT2,
+	MAX8998_CHARGER,
 };
 
 /**
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v2 19/19] dt-bindings: spi: Convert DW SPI binding to DT schema
From: Serge Semin @ 2020-05-16 20:59 UTC (permalink / raw)
  To: Mark Brown, Rob Herring
  Cc: Serge Semin, Georgy Vlasov, Ramil Zaripov, Alexey Malahov,
	Thomas Bogendoerfer, Paul Burton, Ralf Baechle, Arnd Bergmann,
	Allison Randal, Andy Shevchenko, Gareth Williams, linux-mips,
	Wan Ahmad Zainie, linux-spi, devicetree, linux-kernel
In-Reply-To: <20200515104758.6934-20-Sergey.Semin@baikalelectronics.ru>

Rob,
Could you take a look at this patch? We are getting closer to merging the series
into the spi/for-next branch and then to the kernel 5.8. It would be great to
have the bindings converted by the next merge window.

-Sergey

On Fri, May 15, 2020 at 01:47:58PM +0300, Serge Semin wrote:
> Modern device tree bindings are supposed to be created as YAML-files
> in accordance with dt-schema. This commit replaces two DW SPI legacy
> bare text bindings with YAML file. As before the bindings file states
> that the corresponding dts node is supposed to be compatible either
> with generic DW APB SSI controller or with Microsemi/Amazon/Renesas/Intel
> vendors-specific controllers, to have registers, interrupts and clocks
> properties. Though in case of Microsemi version of the controller
> there must be two registers resources specified. Properties like
> clock-names, reg-io-width, cs-gpio, num-cs, DMA and slave device
> sub-nodes are optional.

[nip]

^ permalink raw reply

* [PATCH 01/12] dt-bindings: display: Convert ingenic,lcd.txt to YAML
From: Paul Cercueil @ 2020-05-16 21:50 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, Rob Herring, Greg Kroah-Hartman,
	Rafael J . Wysocki
  Cc: od, dri-devel, devicetree, linux-kernel, Paul Cercueil

Convert the ingenic,lcd.txt to a new ingenic,lcd.yaml file.

In the process, the new ingenic,jz4780-lcd compatible string has been
added.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---

Notes:
    This patch comes from a different patchset so it's effectively a V2.
    
    Changes were:
    - lcd_pclk and lcd clocks are in the correct order now,
    - Add 'port' and 'ports' properties, and document the valid ports.

 .../bindings/display/ingenic,lcd.txt          |  45 -------
 .../bindings/display/ingenic,lcd.yaml         | 126 ++++++++++++++++++
 2 files changed, 126 insertions(+), 45 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/display/ingenic,lcd.txt
 create mode 100644 Documentation/devicetree/bindings/display/ingenic,lcd.yaml

diff --git a/Documentation/devicetree/bindings/display/ingenic,lcd.txt b/Documentation/devicetree/bindings/display/ingenic,lcd.txt
deleted file mode 100644
index 01e3261defb6..000000000000
--- a/Documentation/devicetree/bindings/display/ingenic,lcd.txt
+++ /dev/null
@@ -1,45 +0,0 @@
-Ingenic JZ47xx LCD driver
-
-Required properties:
-- compatible: one of:
-  * ingenic,jz4740-lcd
-  * ingenic,jz4725b-lcd
-  * ingenic,jz4770-lcd
-- reg: LCD registers location and length
-- clocks: LCD pixclock and device clock specifiers.
-	   The device clock is only required on the JZ4740.
-- clock-names: "lcd_pclk" and "lcd"
-- interrupts: Specifies the interrupt line the LCD controller is connected to.
-
-Example:
-
-panel {
-	compatible = "sharp,ls020b1dd01d";
-
-	backlight = <&backlight>;
-	power-supply = <&vcc>;
-
-	port {
-		panel_input: endpoint {
-			remote-endpoint = <&panel_output>;
-		};
-	};
-};
-
-
-lcd: lcd-controller@13050000 {
-	compatible = "ingenic,jz4725b-lcd";
-	reg = <0x13050000 0x1000>;
-
-	interrupt-parent = <&intc>;
-	interrupts = <31>;
-
-	clocks = <&cgu JZ4725B_CLK_LCD>;
-	clock-names = "lcd";
-
-	port {
-		panel_output: endpoint {
-			remote-endpoint = <&panel_input>;
-		};
-	};
-};
diff --git a/Documentation/devicetree/bindings/display/ingenic,lcd.yaml b/Documentation/devicetree/bindings/display/ingenic,lcd.yaml
new file mode 100644
index 000000000000..d56db1802fad
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/ingenic,lcd.yaml
@@ -0,0 +1,126 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/ingenic,lcd.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ingenic SoCs LCD controller devicetree bindings
+
+maintainers:
+  - Paul Cercueil <paul@crapouillou.net>
+
+properties:
+  $nodename:
+    pattern: "^lcd-controller@[0-9a-f]+$"
+
+  compatible:
+    enum:
+      - ingenic,jz4740-lcd
+      - ingenic,jz4725b-lcd
+      - ingenic,jz4770-lcd
+      - ingenic,jz4780-lcd
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Pixel clock
+      - description: Module clock
+    minItems: 1
+
+  clock-names:
+    items:
+      - const: lcd_pclk
+      - const: lcd
+    minItems: 1
+
+  port:
+    description: OF graph bindings (specified in bindings/graph.txt).
+
+  ports:
+    description: OF graph bindings (specified in bindings/graph.txt).
+    type: object
+    properties:
+      port@0:
+        type: object
+        description: DPI output, to interface with TFT panels.
+
+      port@8:
+        type: object
+        description: Link to the Image Processing Unit (IPU).
+          (See ingenic,ipu.yaml).
+
+    required:
+      - port@0
+
+required:
+    - compatible
+    - reg
+    - interrupts
+    - clocks
+    - clock-names
+
+if:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - ingenic,jz4740-lcd
+          - ingenic,jz4780-lcd
+then:
+  properties:
+    clocks:
+      minItems: 2
+    clock-names:
+      minItems: 2
+else:
+  properties:
+    clocks:
+      maxItems: 1
+    clock-names:
+      maxItems: 1
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/jz4740-cgu.h>
+    lcd-controller@13050000 {
+      compatible = "ingenic,jz4740-lcd";
+      reg = <0x13050000 0x1000>;
+
+      interrupt-parent = <&intc>;
+      interrupts = <30>;
+
+      clocks = <&cgu JZ4740_CLK_LCD_PCLK>, <&cgu JZ4740_CLK_LCD>;
+      clock-names = "lcd_pclk", "lcd";
+
+      port {
+        endpoint {
+          remote-endpoint = <&panel_input>;
+        };
+      };
+    };
+
+  - |
+    #include <dt-bindings/clock/jz4725b-cgu.h>
+    lcd-controller@13050000 {
+      compatible = "ingenic,jz4725b-lcd";
+      reg = <0x13050000 0x1000>;
+
+      interrupt-parent = <&intc>;
+      interrupts = <31>;
+
+      clocks = <&cgu JZ4725B_CLK_LCD>;
+      clock-names = "lcd_pclk";
+
+      port {
+        endpoint {
+          remote-endpoint = <&panel_input>;
+        };
+      };
+    };
-- 
2.26.2


^ permalink raw reply related

* Re: [PATCH v9 2/4] media: i2c: Add MAX9286 driver
From: Sakari Ailus @ 2020-05-16 21:51 UTC (permalink / raw)
  To: Kieran Bingham
  Cc: linux-renesas-soc, linux-media, devicetree, linux-kernel,
	Mauro Carvalho Chehab, Kieran Bingham, Laurent Pinchart,
	Jacopo Mondi, Niklas Söderlund, Hans Verkuil, Hyun Kwon,
	Manivannan Sadhasivam, Rob Herring, Jacopo Mondi,
	Laurent Pinchart, Niklas Söderlund
In-Reply-To: <20200512155105.1068064-3-kieran.bingham+renesas@ideasonboard.com>

Hi Kieran,

Thanks for the update.

On Tue, May 12, 2020 at 04:51:03PM +0100, Kieran Bingham wrote:

...

> +static int max9286_enum_mbus_code(struct v4l2_subdev *sd,
> +				  struct v4l2_subdev_pad_config *cfg,
> +				  struct v4l2_subdev_mbus_code_enum *code)
> +{
> +	if (code->pad || code->index > 0)
> +		return -EINVAL;
> +
> +	code->code = MEDIA_BUS_FMT_UYVY8_2X8;

Why UYVY8_2X8 and not UYVY8_1X16? In general, the single sample / pixel
variant of the format is generally used on the serial busses. This choice
was made when serial busses were introduced.

> +
> +	return 0;
> +}
> +
> +static struct v4l2_mbus_framefmt *
> +max9286_get_pad_format(struct max9286_priv *priv,
> +		       struct v4l2_subdev_pad_config *cfg,
> +		       unsigned int pad, u32 which)
> +{
> +	switch (which) {
> +	case V4L2_SUBDEV_FORMAT_TRY:
> +		return v4l2_subdev_get_try_format(&priv->sd, cfg, pad);
> +	case V4L2_SUBDEV_FORMAT_ACTIVE:
> +		return &priv->fmt[pad];
> +	default:
> +		return NULL;
> +	}
> +}
> +
> +static int max9286_set_fmt(struct v4l2_subdev *sd,
> +			   struct v4l2_subdev_pad_config *cfg,
> +			   struct v4l2_subdev_format *format)
> +{
> +	struct max9286_priv *priv = sd_to_max9286(sd);
> +	struct v4l2_mbus_framefmt *cfg_fmt;
> +
> +	if (format->pad >= MAX9286_SRC_PAD)
> +		return -EINVAL;

You can remove these checks; it's been already done by the caller.

...

> +static int max9286_parse_dt(struct max9286_priv *priv)
> +{
> +	struct device *dev = &priv->client->dev;
> +	struct device_node *i2c_mux;
> +	struct device_node *node = NULL;
> +	unsigned int i2c_mux_mask = 0;
> +
> +	of_node_get(dev->of_node);
> +	i2c_mux = of_find_node_by_name(dev->of_node, "i2c-mux");
> +	if (!i2c_mux) {
> +		dev_err(dev, "Failed to find i2c-mux node\n");
> +		of_node_put(dev->of_node);
> +		return -EINVAL;
> +	}
> +
> +	/* Identify which i2c-mux channels are enabled */
> +	for_each_child_of_node(i2c_mux, node) {
> +		u32 id = 0;
> +
> +		of_property_read_u32(node, "reg", &id);
> +		if (id >= MAX9286_NUM_GMSL)
> +			continue;
> +
> +		if (!of_device_is_available(node)) {
> +			dev_dbg(dev, "Skipping disabled I2C bus port %u\n", id);
> +			continue;
> +		}
> +
> +		i2c_mux_mask |= BIT(id);
> +	}
> +	of_node_put(node);
> +	of_node_put(i2c_mux);
> +
> +	/* Parse the endpoints */
> +	for_each_endpoint_of_node(dev->of_node, node) {
> +		struct max9286_source *source;
> +		struct of_endpoint ep;
> +
> +		of_graph_parse_endpoint(node, &ep);
> +		dev_dbg(dev, "Endpoint %pOF on port %d",
> +			ep.local_node, ep.port);
> +
> +		if (ep.port > MAX9286_NUM_GMSL) {
> +			dev_err(dev, "Invalid endpoint %s on port %d",
> +				of_node_full_name(ep.local_node), ep.port);
> +			continue;
> +		}
> +
> +		/* For the source endpoint just parse the bus configuration. */
> +		if (ep.port == MAX9286_SRC_PAD) {
> +			struct v4l2_fwnode_endpoint vep = {
> +				.bus_type = V4L2_MBUS_CSI2_DPHY
> +			};
> +			int ret;
> +
> +			ret = v4l2_fwnode_endpoint_parse(
> +					of_fwnode_handle(node), &vep);
> +			if (ret) {
> +				of_node_put(node);
> +				of_node_put(dev->of_node);
> +				return ret;
> +			}
> +
> +			if (vep.bus_type != V4L2_MBUS_CSI2_DPHY) {

This won't happen, the bus type will stay if you set it to a non-zero
value.

> +				dev_err(dev,
> +					"Media bus %u type not supported\n",
> +					vep.bus_type);
> +				v4l2_fwnode_endpoint_free(&vep);
> +				of_node_put(node);
> +				of_node_put(dev->of_node);
> +				return -EINVAL;
> +			}
> +
> +			priv->csi2_data_lanes =
> +				vep.bus.mipi_csi2.num_data_lanes;
> +			v4l2_fwnode_endpoint_free(&vep);

No need to call this unless you use v4l2_fwnode_endpoint_alloc_parse().

And as you don't, you also won't know which frequencies are known to be
safe to use. That said, perhaps where this device is used having a random
frequency on that bus could not be an issue. Perhaps.

> +
> +			continue;
> +		}
> +
> +		/* Skip if the corresponding GMSL link is unavailable. */
> +		if (!(i2c_mux_mask & BIT(ep.port)))
> +			continue;
> +
> +		if (priv->sources[ep.port].fwnode) {
> +			dev_err(dev,
> +				"Multiple port endpoints are not supported: %d",
> +				ep.port);
> +
> +			continue;
> +		}
> +
> +		source = &priv->sources[ep.port];
> +		source->fwnode = fwnode_graph_get_remote_endpoint(
> +						of_fwnode_handle(node));
> +		if (!source->fwnode) {
> +			dev_err(dev,
> +				"Endpoint %pOF has no remote endpoint connection\n",
> +				ep.local_node);
> +
> +			continue;
> +		}
> +
> +		priv->source_mask |= BIT(ep.port);
> +		priv->nsources++;
> +	}
> +	of_node_put(node);
> +	of_node_put(dev->of_node);
> +
> +	priv->route_mask = priv->source_mask;
> +
> +	return 0;
> +}

-- 
Kind regards,

Sakari Ailus

^ permalink raw reply

* [PATCH 02/12] dt-bindings: display: Add ingenic,ipu.yaml
From: Paul Cercueil @ 2020-05-16 21:50 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, Rob Herring, Greg Kroah-Hartman,
	Rafael J . Wysocki
  Cc: od, dri-devel, devicetree, linux-kernel, Paul Cercueil
In-Reply-To: <20200516215057.392609-1-paul@crapouillou.net>

Add documentation of the Device Tree bindings for the Image Processing
Unit (IPU) found in most Ingenic SoCs.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 .../bindings/display/ingenic,ipu.yaml         | 65 +++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/ingenic,ipu.yaml

diff --git a/Documentation/devicetree/bindings/display/ingenic,ipu.yaml b/Documentation/devicetree/bindings/display/ingenic,ipu.yaml
new file mode 100644
index 000000000000..22fe02ca866d
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/ingenic,ipu.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/ingenic,ipu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ingenic SoCs Image Processing Unit (IPU) devicetree bindings
+
+maintainers:
+  - Paul Cercueil <paul@crapouillou.net>
+
+properties:
+  compatible:
+    oneOf:
+      - enum:
+        - ingenic,jz4725b-ipu
+        - ingenic,jz4760-ipu
+      - items:
+        - ingenic,jz4770-ipu
+        - ingenic,jz4760-ipu
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: ipu
+
+patternProperties:
+  "^ports?$":
+    description: OF graph bindings (specified in bindings/graph.txt).
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/jz4770-cgu.h>
+    ipu@13080000 {
+      compatible = "ingenic,jz4770-ipu", "ingenic,jz4760-ipu";
+      reg = <0x13080000 0x800>;
+
+      interrupt-parent = <&intc>;
+      interrupts = <29>;
+
+      clocks = <&cgu JZ4770_CLK_IPU>;
+      clock-names = "ipu";
+
+      port {
+        ipu_ep: endpoint {
+          remote-endpoint = <&lcdc_ep>;
+        };
+      };
+    };
-- 
2.26.2


^ permalink raw reply related

* [PATCH 03/12] component: Support binding with no matches
From: Paul Cercueil @ 2020-05-16 21:50 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, Rob Herring, Greg Kroah-Hartman,
	Rafael J . Wysocki
  Cc: od, dri-devel, devicetree, linux-kernel, Paul Cercueil
In-Reply-To: <20200516215057.392609-1-paul@crapouillou.net>

Support binding the master even though no components have been
registered.

This permits to support cases where components are optional.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/base/component.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/base/component.c b/drivers/base/component.c
index e97704104784..a9de7ee1677f 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -100,7 +100,7 @@ static int component_devices_show(struct seq_file *s, void *data)
 
 	seq_printf(s, "%-40s %20s\n", "device name", "status");
 	seq_puts(s, "-------------------------------------------------------------\n");
-	for (i = 0; i < match->num; i++) {
+	for (i = 0; !!match && i < match->num; i++) {
 		struct component *component = match->compare[i].component;
 
 		seq_printf(s, "%-40s %20s\n",
@@ -184,6 +184,11 @@ static int find_components(struct master *master)
 	size_t i;
 	int ret = 0;
 
+	if (!match) {
+		dev_dbg(master->dev, "No components\n");
+		return 0;
+	}
+
 	/*
 	 * Scan the array of match functions and attach
 	 * any components which are found to this master.
@@ -218,10 +223,12 @@ static void remove_component(struct master *master, struct component *c)
 {
 	size_t i;
 
-	/* Detach the component from this master. */
-	for (i = 0; i < master->match->num; i++)
-		if (master->match->compare[i].component == c)
-			master->match->compare[i].component = NULL;
+	if (master->match) {
+		/* Detach the component from this master. */
+		for (i = 0; i < master->match->num; i++)
+			if (master->match->compare[i].component == c)
+				master->match->compare[i].component = NULL;
+	}
 }
 
 /*
@@ -470,10 +477,12 @@ int component_master_add_with_match(struct device *dev,
 	struct master *master;
 	int ret;
 
-	/* Reallocate the match array for its true size */
-	ret = component_match_realloc(dev, match, match->num);
-	if (ret)
-		return ret;
+	if (match) {
+		/* Reallocate the match array for its true size */
+		ret = component_match_realloc(dev, match, match->num);
+		if (ret)
+			return ret;
+	}
 
 	master = kzalloc(sizeof(*master), GFP_KERNEL);
 	if (!master)
@@ -557,6 +566,10 @@ void component_unbind_all(struct device *master_dev, void *data)
 	if (!master)
 		return;
 
+	/* No match, nothing to unbind */
+	if (!master->match)
+		return;
+
 	/* Unbind components in reverse order */
 	for (i = master->match->num; i--; )
 		if (!master->match->compare[i].duplicate) {
@@ -640,6 +653,10 @@ int component_bind_all(struct device *master_dev, void *data)
 	if (!master)
 		return -EINVAL;
 
+	/* No match, nothing to bind */
+	if (!master->match)
+		return 0;
+
 	/* Bind components in match order */
 	for (i = 0; i < master->match->num; i++)
 		if (!master->match->compare[i].duplicate) {
-- 
2.26.2


^ permalink raw reply related

* [PATCH 04/12] gpu/drm: ingenic: Fix bogus crtc_atomic_check callback
From: Paul Cercueil @ 2020-05-16 21:50 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, Rob Herring, Greg Kroah-Hartman,
	Rafael J . Wysocki
  Cc: od, dri-devel, devicetree, linux-kernel, Paul Cercueil, stable
In-Reply-To: <20200516215057.392609-1-paul@crapouillou.net>

The code was comparing the SoC's maximum height with the mode's width,
and vice-versa. D'oh.

Cc: stable@vger.kernel.org # v5.6
Fixes: a7c909b7c037 ("gpu/drm: ingenic: Check for display size in CRTC atomic check")
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---

Notes:
    This patch was previously sent standalone.
    I marked it as superseded in patchwork.
    Nothing has been changed here.

 drivers/gpu/drm/ingenic/ingenic-drm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c b/drivers/gpu/drm/ingenic/ingenic-drm.c
index 632d72177123..0c472382a08b 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
@@ -330,8 +330,8 @@ static int ingenic_drm_crtc_atomic_check(struct drm_crtc *crtc,
 	if (!drm_atomic_crtc_needs_modeset(state))
 		return 0;
 
-	if (state->mode.hdisplay > priv->soc_info->max_height ||
-	    state->mode.vdisplay > priv->soc_info->max_width)
+	if (state->mode.hdisplay > priv->soc_info->max_width ||
+	    state->mode.vdisplay > priv->soc_info->max_height)
 		return -EINVAL;
 
 	rate = clk_round_rate(priv->pix_clk,
-- 
2.26.2


^ permalink raw reply related

* [PATCH 05/12] gpu/drm: Ingenic: Fix opaque pointer casted to wrong type
From: Paul Cercueil @ 2020-05-16 21:50 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, Rob Herring, Greg Kroah-Hartman,
	Rafael J . Wysocki
  Cc: od, dri-devel, devicetree, linux-kernel, Paul Cercueil, stable
In-Reply-To: <20200516215057.392609-1-paul@crapouillou.net>

The opaque pointer passed to the IRQ handler is a pointer to the
drm_device, not a pointer to our ingenic_drm structure.

It still worked, because our ingenic_drm structure contains the
drm_device as its first field, so the pointer received had the same
value, but this was not semantically correct.

Cc: stable@vger.kernel.org # v5.3
Fixes: 90b86fcc47b4 ("DRM: Add KMS driver for the Ingenic JZ47xx SoCs")
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/ingenic/ingenic-drm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c b/drivers/gpu/drm/ingenic/ingenic-drm.c
index 0c472382a08b..97244462599b 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
@@ -476,7 +476,7 @@ static int ingenic_drm_encoder_atomic_check(struct drm_encoder *encoder,
 
 static irqreturn_t ingenic_drm_irq_handler(int irq, void *arg)
 {
-	struct ingenic_drm *priv = arg;
+	struct ingenic_drm *priv = drm_device_get_priv(arg);
 	unsigned int state;
 
 	regmap_read(priv->map, JZ_REG_LCD_STATE, &state);
-- 
2.26.2


^ permalink raw reply related

* [PATCH 06/12] gpu/drm: Ingenic: Fix incorrect assumption about plane->index
From: Paul Cercueil @ 2020-05-16 21:50 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, Rob Herring, Greg Kroah-Hartman,
	Rafael J . Wysocki
  Cc: od, dri-devel, devicetree, linux-kernel, Paul Cercueil, stable
In-Reply-To: <20200516215057.392609-1-paul@crapouillou.net>

plane->index is NOT the index of the color plane in a YUV frame.
Actually, a YUV frame is represented by a single drm_plane, even though
it contains three Y, U, V planes.

Cc: stable@vger.kernel.org # v5.3
Fixes: 90b86fcc47b4 ("DRM: Add KMS driver for the Ingenic JZ47xx SoCs")
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/ingenic/ingenic-drm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c b/drivers/gpu/drm/ingenic/ingenic-drm.c
index 97244462599b..3207105755c9 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
@@ -386,7 +386,7 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane,
 		addr = drm_fb_cma_get_gem_addr(state->fb, state, 0);
 		width = state->src_w >> 16;
 		height = state->src_h >> 16;
-		cpp = state->fb->format->cpp[plane->index];
+		cpp = state->fb->format->cpp[0];
 
 		priv->dma_hwdesc->addr = addr;
 		priv->dma_hwdesc->cmd = width * height * cpp / 4;
-- 
2.26.2


^ permalink raw reply related

* [PATCH 07/12] gpu/drm: ingenic: Set DMA descriptor chain address in probe
From: Paul Cercueil @ 2020-05-16 21:50 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, Rob Herring, Greg Kroah-Hartman,
	Rafael J . Wysocki
  Cc: od, dri-devel, devicetree, linux-kernel, Paul Cercueil
In-Reply-To: <20200516215057.392609-1-paul@crapouillou.net>

The address of the DMA descriptor never changes. It can therefore be set
in the probe function.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/ingenic/ingenic-drm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c b/drivers/gpu/drm/ingenic/ingenic-drm.c
index 3207105755c9..e43318938e9e 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
@@ -358,8 +358,6 @@ static void ingenic_drm_crtc_atomic_flush(struct drm_crtc *crtc,
 		ingenic_drm_crtc_update_ctrl(priv, finfo);
 
 		clk_set_rate(priv->pix_clk, state->adjusted_mode.clock * 1000);
-
-		regmap_write(priv->map, JZ_REG_LCD_DA0, priv->dma_hwdesc->next);
 	}
 
 	if (event) {
@@ -768,6 +766,9 @@ static int ingenic_drm_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* Set address of our DMA descriptor chain */
+	regmap_write(priv->map, JZ_REG_LCD_DA0, priv->dma_hwdesc_phys);
+
 	ret = drm_dev_register(drm, 0);
 	if (ret) {
 		dev_err(dev, "Failed to register DRM driver");
-- 
2.26.2


^ permalink raw reply related

* [PATCH 08/12] gpu/drm: Ingenic: Move register definitions to ingenic-drm.h
From: Paul Cercueil @ 2020-05-16 21:50 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, Rob Herring, Greg Kroah-Hartman,
	Rafael J . Wysocki
  Cc: od, dri-devel, devicetree, linux-kernel, Paul Cercueil
In-Reply-To: <20200516215057.392609-1-paul@crapouillou.net>

Move the register definitions to ingenic-drm.h, to keep ingenic-drm.c
tidy.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/ingenic/ingenic-drm.c | 116 +----------------------
 drivers/gpu/drm/ingenic/ingenic-drm.h | 127 ++++++++++++++++++++++++++
 2 files changed, 129 insertions(+), 114 deletions(-)
 create mode 100644 drivers/gpu/drm/ingenic/ingenic-drm.h

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c b/drivers/gpu/drm/ingenic/ingenic-drm.c
index e43318938e9e..92c63d1303f9 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
@@ -4,6 +4,8 @@
 //
 // Copyright (C) 2019, Paul Cercueil <paul@crapouillou.net>
 
+#include "ingenic-drm.h"
+
 #include <linux/clk.h>
 #include <linux/dma-mapping.h>
 #include <linux/module.h>
@@ -32,120 +34,6 @@
 #include <drm/drm_simple_kms_helper.h>
 #include <drm/drm_vblank.h>
 
-#define JZ_REG_LCD_CFG				0x00
-#define JZ_REG_LCD_VSYNC			0x04
-#define JZ_REG_LCD_HSYNC			0x08
-#define JZ_REG_LCD_VAT				0x0C
-#define JZ_REG_LCD_DAH				0x10
-#define JZ_REG_LCD_DAV				0x14
-#define JZ_REG_LCD_PS				0x18
-#define JZ_REG_LCD_CLS				0x1C
-#define JZ_REG_LCD_SPL				0x20
-#define JZ_REG_LCD_REV				0x24
-#define JZ_REG_LCD_CTRL				0x30
-#define JZ_REG_LCD_STATE			0x34
-#define JZ_REG_LCD_IID				0x38
-#define JZ_REG_LCD_DA0				0x40
-#define JZ_REG_LCD_SA0				0x44
-#define JZ_REG_LCD_FID0				0x48
-#define JZ_REG_LCD_CMD0				0x4C
-#define JZ_REG_LCD_DA1				0x50
-#define JZ_REG_LCD_SA1				0x54
-#define JZ_REG_LCD_FID1				0x58
-#define JZ_REG_LCD_CMD1				0x5C
-
-#define JZ_LCD_CFG_SLCD				BIT(31)
-#define JZ_LCD_CFG_PS_DISABLE			BIT(23)
-#define JZ_LCD_CFG_CLS_DISABLE			BIT(22)
-#define JZ_LCD_CFG_SPL_DISABLE			BIT(21)
-#define JZ_LCD_CFG_REV_DISABLE			BIT(20)
-#define JZ_LCD_CFG_HSYNCM			BIT(19)
-#define JZ_LCD_CFG_PCLKM			BIT(18)
-#define JZ_LCD_CFG_INV				BIT(17)
-#define JZ_LCD_CFG_SYNC_DIR			BIT(16)
-#define JZ_LCD_CFG_PS_POLARITY			BIT(15)
-#define JZ_LCD_CFG_CLS_POLARITY			BIT(14)
-#define JZ_LCD_CFG_SPL_POLARITY			BIT(13)
-#define JZ_LCD_CFG_REV_POLARITY			BIT(12)
-#define JZ_LCD_CFG_HSYNC_ACTIVE_LOW		BIT(11)
-#define JZ_LCD_CFG_PCLK_FALLING_EDGE		BIT(10)
-#define JZ_LCD_CFG_DE_ACTIVE_LOW		BIT(9)
-#define JZ_LCD_CFG_VSYNC_ACTIVE_LOW		BIT(8)
-#define JZ_LCD_CFG_18_BIT			BIT(7)
-#define JZ_LCD_CFG_PDW				(BIT(5) | BIT(4))
-
-#define JZ_LCD_CFG_MODE_GENERIC_16BIT		0
-#define JZ_LCD_CFG_MODE_GENERIC_18BIT		BIT(7)
-#define JZ_LCD_CFG_MODE_GENERIC_24BIT		BIT(6)
-
-#define JZ_LCD_CFG_MODE_SPECIAL_TFT_1		1
-#define JZ_LCD_CFG_MODE_SPECIAL_TFT_2		2
-#define JZ_LCD_CFG_MODE_SPECIAL_TFT_3		3
-
-#define JZ_LCD_CFG_MODE_TV_OUT_P		4
-#define JZ_LCD_CFG_MODE_TV_OUT_I		6
-
-#define JZ_LCD_CFG_MODE_SINGLE_COLOR_STN	8
-#define JZ_LCD_CFG_MODE_SINGLE_MONOCHROME_STN	9
-#define JZ_LCD_CFG_MODE_DUAL_COLOR_STN		10
-#define JZ_LCD_CFG_MODE_DUAL_MONOCHROME_STN	11
-
-#define JZ_LCD_CFG_MODE_8BIT_SERIAL		12
-#define JZ_LCD_CFG_MODE_LCM			13
-
-#define JZ_LCD_VSYNC_VPS_OFFSET			16
-#define JZ_LCD_VSYNC_VPE_OFFSET			0
-
-#define JZ_LCD_HSYNC_HPS_OFFSET			16
-#define JZ_LCD_HSYNC_HPE_OFFSET			0
-
-#define JZ_LCD_VAT_HT_OFFSET			16
-#define JZ_LCD_VAT_VT_OFFSET			0
-
-#define JZ_LCD_DAH_HDS_OFFSET			16
-#define JZ_LCD_DAH_HDE_OFFSET			0
-
-#define JZ_LCD_DAV_VDS_OFFSET			16
-#define JZ_LCD_DAV_VDE_OFFSET			0
-
-#define JZ_LCD_CTRL_BURST_4			(0x0 << 28)
-#define JZ_LCD_CTRL_BURST_8			(0x1 << 28)
-#define JZ_LCD_CTRL_BURST_16			(0x2 << 28)
-#define JZ_LCD_CTRL_RGB555			BIT(27)
-#define JZ_LCD_CTRL_OFUP			BIT(26)
-#define JZ_LCD_CTRL_FRC_GRAYSCALE_16		(0x0 << 24)
-#define JZ_LCD_CTRL_FRC_GRAYSCALE_4		(0x1 << 24)
-#define JZ_LCD_CTRL_FRC_GRAYSCALE_2		(0x2 << 24)
-#define JZ_LCD_CTRL_PDD_MASK			(0xff << 16)
-#define JZ_LCD_CTRL_EOF_IRQ			BIT(13)
-#define JZ_LCD_CTRL_SOF_IRQ			BIT(12)
-#define JZ_LCD_CTRL_OFU_IRQ			BIT(11)
-#define JZ_LCD_CTRL_IFU0_IRQ			BIT(10)
-#define JZ_LCD_CTRL_IFU1_IRQ			BIT(9)
-#define JZ_LCD_CTRL_DD_IRQ			BIT(8)
-#define JZ_LCD_CTRL_QDD_IRQ			BIT(7)
-#define JZ_LCD_CTRL_REVERSE_ENDIAN		BIT(6)
-#define JZ_LCD_CTRL_LSB_FISRT			BIT(5)
-#define JZ_LCD_CTRL_DISABLE			BIT(4)
-#define JZ_LCD_CTRL_ENABLE			BIT(3)
-#define JZ_LCD_CTRL_BPP_1			0x0
-#define JZ_LCD_CTRL_BPP_2			0x1
-#define JZ_LCD_CTRL_BPP_4			0x2
-#define JZ_LCD_CTRL_BPP_8			0x3
-#define JZ_LCD_CTRL_BPP_15_16			0x4
-#define JZ_LCD_CTRL_BPP_18_24			0x5
-#define JZ_LCD_CTRL_BPP_MASK			(JZ_LCD_CTRL_RGB555 | (0x7 << 0))
-
-#define JZ_LCD_CMD_SOF_IRQ			BIT(31)
-#define JZ_LCD_CMD_EOF_IRQ			BIT(30)
-#define JZ_LCD_CMD_ENABLE_PAL			BIT(28)
-
-#define JZ_LCD_SYNC_MASK			0x3ff
-
-#define JZ_LCD_STATE_EOF_IRQ			BIT(5)
-#define JZ_LCD_STATE_SOF_IRQ			BIT(4)
-#define JZ_LCD_STATE_DISABLED			BIT(0)
-
 struct ingenic_dma_hwdesc {
 	u32 next;
 	u32 addr;
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.h b/drivers/gpu/drm/ingenic/ingenic-drm.h
new file mode 100644
index 000000000000..4186708a67bc
--- /dev/null
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.h
@@ -0,0 +1,127 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Ingenic JZ47xx KMS driver - Register definitions and private API
+ *
+ * Copyright (C) 2020, Paul Cercueil <paul@crapouillou.net>
+ */
+
+#ifndef DRIVERS_GPU_DRM_INGENIC_INGENIC_DRM_H
+#define DRIVERS_GPU_DRM_INGENIC_INGENIC_DRM_H
+
+#include <linux/bitops.h>
+
+#define JZ_REG_LCD_CFG				0x00
+#define JZ_REG_LCD_VSYNC			0x04
+#define JZ_REG_LCD_HSYNC			0x08
+#define JZ_REG_LCD_VAT				0x0C
+#define JZ_REG_LCD_DAH				0x10
+#define JZ_REG_LCD_DAV				0x14
+#define JZ_REG_LCD_PS				0x18
+#define JZ_REG_LCD_CLS				0x1C
+#define JZ_REG_LCD_SPL				0x20
+#define JZ_REG_LCD_REV				0x24
+#define JZ_REG_LCD_CTRL				0x30
+#define JZ_REG_LCD_STATE			0x34
+#define JZ_REG_LCD_IID				0x38
+#define JZ_REG_LCD_DA0				0x40
+#define JZ_REG_LCD_SA0				0x44
+#define JZ_REG_LCD_FID0				0x48
+#define JZ_REG_LCD_CMD0				0x4C
+#define JZ_REG_LCD_DA1				0x50
+#define JZ_REG_LCD_SA1				0x54
+#define JZ_REG_LCD_FID1				0x58
+#define JZ_REG_LCD_CMD1				0x5C
+
+#define JZ_LCD_CFG_SLCD				BIT(31)
+#define JZ_LCD_CFG_PS_DISABLE			BIT(23)
+#define JZ_LCD_CFG_CLS_DISABLE			BIT(22)
+#define JZ_LCD_CFG_SPL_DISABLE			BIT(21)
+#define JZ_LCD_CFG_REV_DISABLE			BIT(20)
+#define JZ_LCD_CFG_HSYNCM			BIT(19)
+#define JZ_LCD_CFG_PCLKM			BIT(18)
+#define JZ_LCD_CFG_INV				BIT(17)
+#define JZ_LCD_CFG_SYNC_DIR			BIT(16)
+#define JZ_LCD_CFG_PS_POLARITY			BIT(15)
+#define JZ_LCD_CFG_CLS_POLARITY			BIT(14)
+#define JZ_LCD_CFG_SPL_POLARITY			BIT(13)
+#define JZ_LCD_CFG_REV_POLARITY			BIT(12)
+#define JZ_LCD_CFG_HSYNC_ACTIVE_LOW		BIT(11)
+#define JZ_LCD_CFG_PCLK_FALLING_EDGE		BIT(10)
+#define JZ_LCD_CFG_DE_ACTIVE_LOW		BIT(9)
+#define JZ_LCD_CFG_VSYNC_ACTIVE_LOW		BIT(8)
+#define JZ_LCD_CFG_18_BIT			BIT(7)
+#define JZ_LCD_CFG_PDW				(BIT(5) | BIT(4))
+
+#define JZ_LCD_CFG_MODE_GENERIC_16BIT		0
+#define JZ_LCD_CFG_MODE_GENERIC_18BIT		BIT(7)
+#define JZ_LCD_CFG_MODE_GENERIC_24BIT		BIT(6)
+
+#define JZ_LCD_CFG_MODE_SPECIAL_TFT_1		1
+#define JZ_LCD_CFG_MODE_SPECIAL_TFT_2		2
+#define JZ_LCD_CFG_MODE_SPECIAL_TFT_3		3
+
+#define JZ_LCD_CFG_MODE_TV_OUT_P		4
+#define JZ_LCD_CFG_MODE_TV_OUT_I		6
+
+#define JZ_LCD_CFG_MODE_SINGLE_COLOR_STN	8
+#define JZ_LCD_CFG_MODE_SINGLE_MONOCHROME_STN	9
+#define JZ_LCD_CFG_MODE_DUAL_COLOR_STN		10
+#define JZ_LCD_CFG_MODE_DUAL_MONOCHROME_STN	11
+
+#define JZ_LCD_CFG_MODE_8BIT_SERIAL		12
+#define JZ_LCD_CFG_MODE_LCM			13
+
+#define JZ_LCD_VSYNC_VPS_OFFSET			16
+#define JZ_LCD_VSYNC_VPE_OFFSET			0
+
+#define JZ_LCD_HSYNC_HPS_OFFSET			16
+#define JZ_LCD_HSYNC_HPE_OFFSET			0
+
+#define JZ_LCD_VAT_HT_OFFSET			16
+#define JZ_LCD_VAT_VT_OFFSET			0
+
+#define JZ_LCD_DAH_HDS_OFFSET			16
+#define JZ_LCD_DAH_HDE_OFFSET			0
+
+#define JZ_LCD_DAV_VDS_OFFSET			16
+#define JZ_LCD_DAV_VDE_OFFSET			0
+
+#define JZ_LCD_CTRL_BURST_4			(0x0 << 28)
+#define JZ_LCD_CTRL_BURST_8			(0x1 << 28)
+#define JZ_LCD_CTRL_BURST_16			(0x2 << 28)
+#define JZ_LCD_CTRL_RGB555			BIT(27)
+#define JZ_LCD_CTRL_OFUP			BIT(26)
+#define JZ_LCD_CTRL_FRC_GRAYSCALE_16		(0x0 << 24)
+#define JZ_LCD_CTRL_FRC_GRAYSCALE_4		(0x1 << 24)
+#define JZ_LCD_CTRL_FRC_GRAYSCALE_2		(0x2 << 24)
+#define JZ_LCD_CTRL_PDD_MASK			(0xff << 16)
+#define JZ_LCD_CTRL_EOF_IRQ			BIT(13)
+#define JZ_LCD_CTRL_SOF_IRQ			BIT(12)
+#define JZ_LCD_CTRL_OFU_IRQ			BIT(11)
+#define JZ_LCD_CTRL_IFU0_IRQ			BIT(10)
+#define JZ_LCD_CTRL_IFU1_IRQ			BIT(9)
+#define JZ_LCD_CTRL_DD_IRQ			BIT(8)
+#define JZ_LCD_CTRL_QDD_IRQ			BIT(7)
+#define JZ_LCD_CTRL_REVERSE_ENDIAN		BIT(6)
+#define JZ_LCD_CTRL_LSB_FISRT			BIT(5)
+#define JZ_LCD_CTRL_DISABLE			BIT(4)
+#define JZ_LCD_CTRL_ENABLE			BIT(3)
+#define JZ_LCD_CTRL_BPP_1			0x0
+#define JZ_LCD_CTRL_BPP_2			0x1
+#define JZ_LCD_CTRL_BPP_4			0x2
+#define JZ_LCD_CTRL_BPP_8			0x3
+#define JZ_LCD_CTRL_BPP_15_16			0x4
+#define JZ_LCD_CTRL_BPP_18_24			0x5
+#define JZ_LCD_CTRL_BPP_MASK			(JZ_LCD_CTRL_RGB555 | 0x7)
+
+#define JZ_LCD_CMD_SOF_IRQ			BIT(31)
+#define JZ_LCD_CMD_EOF_IRQ			BIT(30)
+#define JZ_LCD_CMD_ENABLE_PAL			BIT(28)
+
+#define JZ_LCD_SYNC_MASK			0x3ff
+
+#define JZ_LCD_STATE_EOF_IRQ			BIT(5)
+#define JZ_LCD_STATE_SOF_IRQ			BIT(4)
+#define JZ_LCD_STATE_DISABLED			BIT(0)
+
+#endif /* DRIVERS_GPU_DRM_INGENIC_INGENIC_DRM_H */
-- 
2.26.2


^ permalink raw reply related

* [PATCH 09/12] gpu/drm: Ingenic: Add support for OSD mode
From: Paul Cercueil @ 2020-05-16 21:50 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, Rob Herring, Greg Kroah-Hartman,
	Rafael J . Wysocki
  Cc: od, dri-devel, devicetree, linux-kernel, Paul Cercueil
In-Reply-To: <20200516215057.392609-1-paul@crapouillou.net>

All Ingenic SoCs starting from the JZ4725B support OSD mode.

In this mode, two separate planes can be used. They can have different
positions and sizes, and one can be overlayed on top of the other.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/ingenic/ingenic-drm.c | 275 +++++++++++++++++++++-----
 drivers/gpu/drm/ingenic/ingenic-drm.h |  35 ++++
 2 files changed, 258 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c b/drivers/gpu/drm/ingenic/ingenic-drm.c
index 92c63d1303f9..f139af5b7a40 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
@@ -43,12 +43,13 @@ struct ingenic_dma_hwdesc {
 
 struct jz_soc_info {
 	bool needs_dev_clk;
+	bool has_osd;
 	unsigned int max_width, max_height;
 };
 
 struct ingenic_drm {
 	struct drm_device drm;
-	struct drm_plane primary;
+	struct drm_plane f0, f1;
 	struct drm_crtc crtc;
 	struct drm_encoder encoder;
 
@@ -57,8 +58,8 @@ struct ingenic_drm {
 	struct clk *lcd_clk, *pix_clk;
 	const struct jz_soc_info *soc_info;
 
-	struct ingenic_dma_hwdesc *dma_hwdesc;
-	dma_addr_t dma_hwdesc_phys;
+	struct ingenic_dma_hwdesc *dma_hwdesc[2];
+	dma_addr_t dma_hwdesc_phys[2];
 
 	bool panel_is_sharp;
 };
@@ -90,7 +91,7 @@ static const struct regmap_config ingenic_drm_regmap_config = {
 	.val_bits = 32,
 	.reg_stride = 4,
 
-	.max_register = JZ_REG_LCD_CMD1,
+	.max_register = JZ_REG_LCD_SIZE1,
 	.writeable_reg = ingenic_drm_writeable_reg,
 };
 
@@ -110,11 +111,6 @@ drm_encoder_get_priv(struct drm_encoder *encoder)
 	return container_of(encoder, struct ingenic_drm, encoder);
 }
 
-static inline struct ingenic_drm *drm_plane_get_priv(struct drm_plane *plane)
-{
-	return container_of(plane, struct ingenic_drm, primary);
-}
-
 static void ingenic_drm_crtc_atomic_enable(struct drm_crtc *crtc,
 					   struct drm_crtc_state *state)
 {
@@ -185,34 +181,17 @@ static void ingenic_drm_crtc_update_timings(struct ingenic_drm *priv,
 		regmap_write(priv->map, JZ_REG_LCD_SPL, hpe << 16 | (hpe + 1));
 		regmap_write(priv->map, JZ_REG_LCD_REV, mode->htotal << 16);
 	}
-}
-
-static void ingenic_drm_crtc_update_ctrl(struct ingenic_drm *priv,
-					 const struct drm_format_info *finfo)
-{
-	unsigned int ctrl = JZ_LCD_CTRL_OFUP | JZ_LCD_CTRL_BURST_16;
-
-	switch (finfo->format) {
-	case DRM_FORMAT_XRGB1555:
-		ctrl |= JZ_LCD_CTRL_RGB555;
-		/* fall-through */
-	case DRM_FORMAT_RGB565:
-		ctrl |= JZ_LCD_CTRL_BPP_15_16;
-		break;
-	case DRM_FORMAT_XRGB8888:
-		ctrl |= JZ_LCD_CTRL_BPP_18_24;
-		break;
-	}
 
 	regmap_update_bits(priv->map, JZ_REG_LCD_CTRL,
-			   JZ_LCD_CTRL_OFUP | JZ_LCD_CTRL_BURST_16 |
-			   JZ_LCD_CTRL_BPP_MASK, ctrl);
+			   JZ_LCD_CTRL_OFUP | JZ_LCD_CTRL_BURST_16,
+			   JZ_LCD_CTRL_OFUP | JZ_LCD_CTRL_BURST_16);
 }
 
 static int ingenic_drm_crtc_atomic_check(struct drm_crtc *crtc,
 					 struct drm_crtc_state *state)
 {
 	struct ingenic_drm *priv = drm_crtc_get_priv(crtc);
+	struct drm_plane_state *f1_state, *f0_state;
 	long rate;
 
 	if (!drm_atomic_crtc_needs_modeset(state))
@@ -227,6 +206,17 @@ static int ingenic_drm_crtc_atomic_check(struct drm_crtc *crtc,
 	if (rate < 0)
 		return rate;
 
+	if (priv->soc_info->has_osd) {
+		f1_state = drm_atomic_get_plane_state(state->state,
+						      &priv->f1);
+		f0_state = drm_atomic_get_plane_state(state->state,
+						      &priv->f0);
+
+		/* If all the planes are disabled, we won't get a VBLANK IRQ */
+		if (!f1_state->fb && !f0_state->fb)
+			state->no_vblank = true;
+	}
+
 	return 0;
 }
 
@@ -236,14 +226,9 @@ static void ingenic_drm_crtc_atomic_flush(struct drm_crtc *crtc,
 	struct ingenic_drm *priv = drm_crtc_get_priv(crtc);
 	struct drm_crtc_state *state = crtc->state;
 	struct drm_pending_vblank_event *event = state->event;
-	struct drm_framebuffer *drm_fb = crtc->primary->state->fb;
-	const struct drm_format_info *finfo;
 
 	if (drm_atomic_crtc_needs_modeset(state)) {
-		finfo = drm_format_info(drm_fb->format->format);
-
 		ingenic_drm_crtc_update_timings(priv, &state->mode);
-		ingenic_drm_crtc_update_ctrl(priv, finfo);
 
 		clk_set_rate(priv->pix_clk, state->adjusted_mode.clock * 1000);
 	}
@@ -260,12 +245,149 @@ static void ingenic_drm_crtc_atomic_flush(struct drm_crtc *crtc,
 	}
 }
 
+static int ingenic_drm_plane_atomic_check(struct drm_plane *plane,
+					  struct drm_plane_state *state)
+{
+	struct ingenic_drm *priv = drm_device_get_priv(plane->dev);
+	struct drm_crtc_state *crtc_state;
+	struct drm_crtc *crtc = state->crtc ?: plane->state->crtc;
+	int ret;
+
+	if (!crtc)
+		return 0;
+
+	crtc_state = drm_atomic_get_existing_crtc_state(state->state, crtc);
+	if (WARN_ON(!crtc_state))
+		return -EINVAL;
+
+	ret = drm_atomic_helper_check_plane_state(state, crtc_state,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  DRM_PLANE_HELPER_NO_SCALING,
+						  priv->soc_info->has_osd,
+						  true);
+	if (ret)
+		return ret;
+
+	if (!priv->soc_info->has_osd &&
+	    (state->src_x != 0 ||
+	     (state->src_w >> 16) != state->crtc_w ||
+	     (state->src_h >> 16) != state->crtc_h))
+		return -EINVAL;
+
+	/*
+	 * Require full modeset if if enabling or disabling a plane, or changing
+	 * its position, size or depth.
+	 */
+	if (priv->soc_info->has_osd &&
+	    (!plane->state->fb || !state->fb ||
+	     plane->state->crtc_x != state->crtc_x ||
+	     plane->state->crtc_y != state->crtc_y ||
+	     plane->state->crtc_w != state->crtc_w ||
+	     plane->state->crtc_h != state->crtc_h ||
+	     plane->state->fb->format->format != state->fb->format->format))
+		crtc_state->mode_changed = true;
+
+	return 0;
+}
+
+static void ingenic_drm_plane_enable(struct ingenic_drm *priv,
+				      struct drm_plane *plane)
+{
+	unsigned int en_bit;
+
+	if (priv->soc_info->has_osd) {
+		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
+			en_bit = JZ_LCD_OSDC_F1EN;
+		else
+			en_bit = JZ_LCD_OSDC_F0EN;
+
+		regmap_update_bits(priv->map, JZ_REG_LCD_OSDC, en_bit, en_bit);
+	}
+}
+
+static void ingenic_drm_plane_atomic_disable(struct drm_plane *plane,
+					     struct drm_plane_state *old_state)
+{
+	struct ingenic_drm *priv = drm_device_get_priv(plane->dev);
+	unsigned int en_bit;
+
+	if (priv->soc_info->has_osd) {
+		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
+			en_bit = JZ_LCD_OSDC_F1EN;
+		else
+			en_bit = JZ_LCD_OSDC_F0EN;
+
+		regmap_update_bits(priv->map, JZ_REG_LCD_OSDC, en_bit, 0);
+	}
+}
+
+static void ingenic_drm_plane_config(struct ingenic_drm *priv,
+				     struct drm_plane *plane, u32 fourcc)
+{
+	struct drm_plane_state *state = plane->state;
+	unsigned int xy_reg, size_reg;
+	unsigned int ctrl = 0;
+
+	ingenic_drm_plane_enable(priv, plane);
+
+	if (priv->soc_info->has_osd &&
+	    plane->type == DRM_PLANE_TYPE_PRIMARY) {
+		switch (fourcc) {
+		case DRM_FORMAT_XRGB1555:
+			ctrl |= JZ_LCD_OSDCTRL_RGB555;
+			fallthrough;
+		case DRM_FORMAT_RGB565:
+			ctrl |= JZ_LCD_OSDCTRL_BPP_15_16;
+			break;
+		case DRM_FORMAT_XRGB8888:
+			ctrl |= JZ_LCD_OSDCTRL_BPP_18_24;
+			break;
+		}
+
+		regmap_update_bits(priv->map, JZ_REG_LCD_OSDCTRL,
+				   JZ_LCD_OSDCTRL_BPP_MASK, ctrl);
+	} else {
+		switch (fourcc) {
+		case DRM_FORMAT_XRGB1555:
+			ctrl |= JZ_LCD_CTRL_RGB555;
+			fallthrough;
+		case DRM_FORMAT_RGB565:
+			ctrl |= JZ_LCD_CTRL_BPP_15_16;
+			break;
+		case DRM_FORMAT_XRGB8888:
+			ctrl |= JZ_LCD_CTRL_BPP_18_24;
+			break;
+		}
+
+		regmap_update_bits(priv->map, JZ_REG_LCD_CTRL,
+				   JZ_LCD_CTRL_BPP_MASK, ctrl);
+	}
+
+	if (priv->soc_info->has_osd) {
+		if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
+			xy_reg = JZ_REG_LCD_XYP1;
+			size_reg = JZ_REG_LCD_SIZE1;
+		} else {
+			xy_reg = JZ_REG_LCD_XYP0;
+			size_reg = JZ_REG_LCD_SIZE0;
+		}
+
+		regmap_write(priv->map, xy_reg,
+			     state->crtc_x << JZ_LCD_XYP01_XPOS_LSB |
+			     state->crtc_y << JZ_LCD_XYP01_YPOS_LSB);
+		regmap_write(priv->map, size_reg,
+			     state->crtc_w << JZ_LCD_SIZE01_WIDTH_LSB |
+			     state->crtc_h << JZ_LCD_SIZE01_HEIGHT_LSB);
+	}
+}
+
 static void ingenic_drm_plane_atomic_update(struct drm_plane *plane,
 					    struct drm_plane_state *oldstate)
 {
-	struct ingenic_drm *priv = drm_plane_get_priv(plane);
+	struct ingenic_drm *priv = drm_device_get_priv(plane->dev);
 	struct drm_plane_state *state = plane->state;
 	unsigned int width, height, cpp;
+	unsigned int hwdesc_idx;
 	dma_addr_t addr;
 
 	if (state && state->fb) {
@@ -274,9 +396,18 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane,
 		height = state->src_h >> 16;
 		cpp = state->fb->format->cpp[0];
 
-		priv->dma_hwdesc->addr = addr;
-		priv->dma_hwdesc->cmd = width * height * cpp / 4;
-		priv->dma_hwdesc->cmd |= JZ_LCD_CMD_EOF_IRQ;
+		if (!priv->soc_info->has_osd)
+			hwdesc_idx = 0;
+		else
+			hwdesc_idx = plane->type == DRM_PLANE_TYPE_PRIMARY;
+
+		priv->dma_hwdesc[hwdesc_idx]->addr = addr;
+		priv->dma_hwdesc[hwdesc_idx]->cmd = width * height * cpp / 4;
+		priv->dma_hwdesc[hwdesc_idx]->cmd |= JZ_LCD_CMD_EOF_IRQ;
+
+		if (drm_atomic_crtc_needs_modeset(state->crtc->state))
+			ingenic_drm_plane_config(priv, plane,
+						 state->fb->format->format);
 	}
 }
 
@@ -448,6 +579,8 @@ static const struct drm_crtc_funcs ingenic_drm_crtc_funcs = {
 
 static const struct drm_plane_helper_funcs ingenic_drm_plane_helper_funcs = {
 	.atomic_update		= ingenic_drm_plane_atomic_update,
+	.atomic_check		= ingenic_drm_plane_atomic_check,
+	.atomic_disable		= ingenic_drm_plane_atomic_disable,
 	.prepare_fb		= drm_gem_fb_prepare_fb,
 };
 
@@ -474,8 +607,10 @@ static void ingenic_drm_free_dma_hwdesc(void *d)
 {
 	struct ingenic_drm *priv = d;
 
-	dma_free_coherent(priv->dev, sizeof(*priv->dma_hwdesc),
-			  priv->dma_hwdesc, priv->dma_hwdesc_phys);
+	dma_free_coherent(priv->dev, sizeof(*priv->dma_hwdesc[0]),
+			  priv->dma_hwdesc[0], priv->dma_hwdesc_phys[0]);
+	dma_free_coherent(priv->dev, sizeof(*priv->dma_hwdesc[1]),
+			  priv->dma_hwdesc[1], priv->dma_hwdesc_phys[1]);
 }
 
 static int ingenic_drm_probe(struct platform_device *pdev)
@@ -560,23 +695,34 @@ static int ingenic_drm_probe(struct platform_device *pdev)
 		bridge = devm_drm_panel_bridge_add_typed(dev, panel,
 							 DRM_MODE_CONNECTOR_DPI);
 
-	priv->dma_hwdesc = dma_alloc_coherent(dev, sizeof(*priv->dma_hwdesc),
-					      &priv->dma_hwdesc_phys,
-					      GFP_KERNEL);
-	if (!priv->dma_hwdesc)
+	priv->dma_hwdesc[0] = dma_alloc_coherent(dev,
+						 sizeof(*priv->dma_hwdesc[0]),
+						 &priv->dma_hwdesc_phys[0],
+						 GFP_KERNEL);
+	if (!priv->dma_hwdesc[0])
+		return -ENOMEM;
+
+	priv->dma_hwdesc[0]->next = priv->dma_hwdesc_phys[0];
+	priv->dma_hwdesc[0]->id = 0xdeafbead;
+
+	priv->dma_hwdesc[1] = dma_alloc_coherent(dev,
+						 sizeof(*priv->dma_hwdesc[1]),
+						 &priv->dma_hwdesc_phys[1],
+						 GFP_KERNEL);
+	if (!priv->dma_hwdesc[1])
 		return -ENOMEM;
 
+	priv->dma_hwdesc[1]->next = priv->dma_hwdesc_phys[1];
+	priv->dma_hwdesc[1]->id = 0xdeadbabe;
+
 	ret = devm_add_action_or_reset(dev, ingenic_drm_free_dma_hwdesc, priv);
 	if (ret)
 		return ret;
 
-	priv->dma_hwdesc->next = priv->dma_hwdesc_phys;
-	priv->dma_hwdesc->id = 0xdeafbead;
-
-	drm_plane_helper_add(&priv->primary, &ingenic_drm_plane_helper_funcs);
+	drm_plane_helper_add(&priv->f1, &ingenic_drm_plane_helper_funcs);
 
-	ret = drm_universal_plane_init(drm, &priv->primary,
-				       0, &ingenic_drm_primary_plane_funcs,
+	ret = drm_universal_plane_init(drm, &priv->f1, 1,
+				       &ingenic_drm_primary_plane_funcs,
 				       ingenic_drm_primary_formats,
 				       ARRAY_SIZE(ingenic_drm_primary_formats),
 				       NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
@@ -587,13 +733,30 @@ static int ingenic_drm_probe(struct platform_device *pdev)
 
 	drm_crtc_helper_add(&priv->crtc, &ingenic_drm_crtc_helper_funcs);
 
-	ret = drm_crtc_init_with_planes(drm, &priv->crtc, &priv->primary,
+	ret = drm_crtc_init_with_planes(drm, &priv->crtc, &priv->f1,
 					NULL, &ingenic_drm_crtc_funcs, NULL);
 	if (ret) {
 		dev_err(dev, "Failed to init CRTC: %i", ret);
 		return ret;
 	}
 
+	if (soc_info->has_osd) {
+		drm_plane_helper_add(&priv->f0,
+				     &ingenic_drm_plane_helper_funcs);
+
+		ret = drm_universal_plane_init(drm, &priv->f0, 1,
+					       &ingenic_drm_primary_plane_funcs,
+					       ingenic_drm_primary_formats,
+					       ARRAY_SIZE(ingenic_drm_primary_formats),
+					       NULL, DRM_PLANE_TYPE_OVERLAY,
+					       NULL);
+		if (ret) {
+			dev_err(dev, "Failed to register overlay plane: %i\n",
+				ret);
+			return ret;
+		}
+	}
+
 	priv->encoder.possible_crtcs = 1;
 
 	drm_encoder_helper_add(&priv->encoder,
@@ -655,7 +818,12 @@ static int ingenic_drm_probe(struct platform_device *pdev)
 	}
 
 	/* Set address of our DMA descriptor chain */
-	regmap_write(priv->map, JZ_REG_LCD_DA0, priv->dma_hwdesc_phys);
+	regmap_write(priv->map, JZ_REG_LCD_DA0, priv->dma_hwdesc_phys[0]);
+	regmap_write(priv->map, JZ_REG_LCD_DA1, priv->dma_hwdesc_phys[1]);
+
+	/* Enable OSD if available */
+	if (soc_info->has_osd)
+		regmap_write(priv->map, JZ_REG_LCD_OSDC, JZ_LCD_OSDC_OSDEN);
 
 	ret = drm_dev_register(drm, 0);
 	if (ret) {
@@ -691,18 +859,21 @@ static int ingenic_drm_remove(struct platform_device *pdev)
 
 static const struct jz_soc_info jz4740_soc_info = {
 	.needs_dev_clk = true,
+	.has_osd = false,
 	.max_width = 800,
 	.max_height = 600,
 };
 
 static const struct jz_soc_info jz4725b_soc_info = {
 	.needs_dev_clk = false,
+	.has_osd = true,
 	.max_width = 800,
 	.max_height = 600,
 };
 
 static const struct jz_soc_info jz4770_soc_info = {
 	.needs_dev_clk = false,
+	.has_osd = true,
 	.max_width = 1280,
 	.max_height = 720,
 };
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.h b/drivers/gpu/drm/ingenic/ingenic-drm.h
index 4186708a67bc..b9d0bb0c9310 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.h
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.h
@@ -31,6 +31,18 @@
 #define JZ_REG_LCD_SA1				0x54
 #define JZ_REG_LCD_FID1				0x58
 #define JZ_REG_LCD_CMD1				0x5C
+#define JZ_REG_LCD_OSDC				0x100
+#define JZ_REG_LCD_OSDCTRL			0x104
+#define JZ_REG_LCD_OSDS				0x108
+#define JZ_REG_LCD_BGC				0x10c
+#define JZ_REG_LCD_KEY0				0x110
+#define JZ_REG_LCD_KEY1				0x114
+#define JZ_REG_LCD_ALPHA			0x118
+#define JZ_REG_LCD_IPUR				0x11c
+#define JZ_REG_LCD_XYP0				0x120
+#define JZ_REG_LCD_XYP1				0x124
+#define JZ_REG_LCD_SIZE0			0x128
+#define JZ_REG_LCD_SIZE1			0x12c
 
 #define JZ_LCD_CFG_SLCD				BIT(31)
 #define JZ_LCD_CFG_PS_DISABLE			BIT(23)
@@ -124,4 +136,27 @@
 #define JZ_LCD_STATE_SOF_IRQ			BIT(4)
 #define JZ_LCD_STATE_DISABLED			BIT(0)
 
+#define JZ_LCD_OSDC_OSDEN			BIT(0)
+#define JZ_LCD_OSDC_F0EN			BIT(3)
+#define JZ_LCD_OSDC_F1EN			BIT(4)
+
+#define JZ_LCD_OSDCTRL_IPU			BIT(15)
+#define JZ_LCD_OSDCTRL_RGB555			BIT(4)
+#define JZ_LCD_OSDCTRL_CHANGE			BIT(3)
+#define JZ_LCD_OSDCTRL_BPP_15_16		0x4
+#define JZ_LCD_OSDCTRL_BPP_18_24		0x5
+#define JZ_LCD_OSDCTRL_BPP_30			0x7
+#define JZ_LCD_OSDCTRL_BPP_MASK			(JZ_LCD_OSDCTRL_RGB555 | 0x7)
+
+#define JZ_LCD_OSDS_READY			BIT(0)
+
+#define JZ_LCD_IPUR_IPUREN			BIT(31)
+#define JZ_LCD_IPUR_IPUR_LSB			0
+
+#define JZ_LCD_XYP01_XPOS_LSB			0
+#define JZ_LCD_XYP01_YPOS_LSB			16
+
+#define JZ_LCD_SIZE01_WIDTH_LSB			0
+#define JZ_LCD_SIZE01_HEIGHT_LSB		16
+
 #endif /* DRIVERS_GPU_DRM_INGENIC_INGENIC_DRM_H */
-- 
2.26.2


^ permalink raw reply related

* [PATCH 10/12] gpu/drm: Ingenic: Register driver as component master
From: Paul Cercueil @ 2020-05-16 21:50 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, Rob Herring, Greg Kroah-Hartman,
	Rafael J . Wysocki
  Cc: od, dri-devel, devicetree, linux-kernel, Paul Cercueil
In-Reply-To: <20200516215057.392609-1-paul@crapouillou.net>

Register the ingenic-drm driver as a component master.

This will later allow to plug optional components to the driver, for
instance to add support for the IPU, or the SLCD module.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/ingenic/ingenic-drm.c | 61 +++++++++++++++++++++++++--
 1 file changed, 57 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c b/drivers/gpu/drm/ingenic/ingenic-drm.c
index f139af5b7a40..b53db3ee001f 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
@@ -6,6 +6,7 @@
 
 #include "ingenic-drm.h"
 
+#include <linux/component.h>
 #include <linux/clk.h>
 #include <linux/dma-mapping.h>
 #include <linux/module.h>
@@ -613,10 +614,17 @@ static void ingenic_drm_free_dma_hwdesc(void *d)
 			  priv->dma_hwdesc[1], priv->dma_hwdesc_phys[1]);
 }
 
-static int ingenic_drm_probe(struct platform_device *pdev)
+static void ingenic_drm_unbind_all(void *d)
 {
+	struct ingenic_drm *priv = d;
+
+	component_unbind_all(priv->dev, &priv->drm);
+}
+
+static int ingenic_drm_bind(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
 	const struct jz_soc_info *soc_info;
-	struct device *dev = &pdev->dev;
 	struct ingenic_drm *priv;
 	struct clk *parent_clk;
 	struct drm_bridge *bridge;
@@ -653,6 +661,17 @@ static int ingenic_drm_probe(struct platform_device *pdev)
 	drm->mode_config.max_height = 4095;
 	drm->mode_config.funcs = &ingenic_drm_mode_config_funcs;
 
+	ret = component_bind_all(dev, drm);
+	if (ret) {
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Failed to bind components: %i", ret);
+		return ret;
+	}
+
+	ret = devm_add_action_or_reset(dev, ingenic_drm_unbind_all, priv);
+	if (ret)
+		return ret;
+
 	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base)) {
 		dev_err(dev, "Failed to get memory resource");
@@ -843,9 +862,14 @@ static int ingenic_drm_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int ingenic_drm_remove(struct platform_device *pdev)
+static int compare_of(struct device *dev, void *data)
+{
+	return dev->of_node == data;
+}
+
+static void ingenic_drm_unbind(struct device *dev)
 {
-	struct ingenic_drm *priv = platform_get_drvdata(pdev);
+	struct ingenic_drm *priv = dev_get_drvdata(dev);
 
 	if (priv->lcd_clk)
 		clk_disable_unprepare(priv->lcd_clk);
@@ -853,6 +877,35 @@ static int ingenic_drm_remove(struct platform_device *pdev)
 
 	drm_dev_unregister(&priv->drm);
 	drm_atomic_helper_shutdown(&priv->drm);
+}
+
+static const struct component_master_ops ingenic_master_ops = {
+	.bind = ingenic_drm_bind,
+	.unbind = ingenic_drm_unbind,
+};
+
+static int ingenic_drm_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct component_match *match = NULL;
+	struct device_node *np;
+	unsigned int i;
+
+	/* Probe components at port address 8 and upwards */
+	for (i = 8; ; i++) {
+		np = of_graph_get_remote_node(dev->of_node, i, 0);
+		if (!np)
+			break;
+
+		drm_of_component_match_add(dev, &match, compare_of, np);
+	}
+
+	return component_master_add_with_match(dev, &ingenic_master_ops, match);
+}
+
+static int ingenic_drm_remove(struct platform_device *pdev)
+{
+	component_master_del(&pdev->dev, &ingenic_master_ops);
 
 	return 0;
 }
-- 
2.26.2


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox