Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 02/14] arm64: Call ARCH_WORKAROUND_2 on transitions between EL0 and EL1
From: Julien Grall @ 2018-05-23  9:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522150648.28297-3-marc.zyngier@arm.com>

Hi Marc,

On 05/22/2018 04:06 PM, Marc Zyngier wrote:
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index ec2ee720e33e..f33e6aed3037 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -18,6 +18,7 @@
>    * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>    */
>   
> +#include <linux/arm-smccc.h>
>   #include <linux/init.h>
>   #include <linux/linkage.h>
>   
> @@ -137,6 +138,18 @@ alternative_else_nop_endif
>   	add	\dst, \dst, #(\sym - .entry.tramp.text)
>   	.endm
>   
> +	// This macro corrupts x0-x3. It is the caller's duty
> +	// to save/restore them if required.

NIT: Shouldn't you use /* ... */ for multi-line comments?

Regardless that:

Reviewed-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

^ permalink raw reply

* [PATCH 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Rajendra Nayak @ 2018-05-23  9:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <3838f17a-2ac8-bf3f-f0b1-f69bbe17629c@nvidia.com>



On 05/23/2018 02:37 PM, Jon Hunter wrote:
> 
> On 23/05/18 07:12, Ulf Hansson wrote:
> 
> ...
> 
>>>>>> Thanks for sending this. Believe it or not this has still been on my to-do list
>>>>>> and so we definitely need a solution for Tegra.
>>>>>>
>>>>>> Looking at the above it appears that additional power-domains exposed as devices
>>>>>> to the client device. So I assume that this means that the drivers for devices
>>>>>> with multiple power-domains will need to call RPM APIs for each of these
>>>>>> additional power-domains. Is that correct?
>>>>>
>>>>> They can, but should not!
>>>>>
>>>>> Instead, the driver shall use device_link_add() and device_link_del(),
>>>>> dynamically, depending on what PM domain that their original device
>>>>> needs for the current running use case.
>>>>>
>>>>> In that way, they keep existing runtime PM deployment, operating on
>>>>> its original device.
>>>>
>>>> OK, sounds good. Any reason why the linking cannot be handled by the above API? Is there a use-case where you would not want it linked?
>>>
>>> I am guessing the linking is what would give the driver the ability to decide which subset of powerdomains it actually wants to control
>>> at any point using runtime PM. If we have cases wherein the driver would want to turn on/off _all_ its associated powerdomains _always_
>>> then a default linking of all would help.
>>
>> First, I think we need to decide on *where* the linking should be
>> done, not at both places, as that would just mess up synchronization
>> of who is responsible for calling the device_link_del() at detach.
>>
>> Second, It would in principle be fine to call device_link_add() and
>> device_link_del() as a part of the attach/detach APIs. However, there
>> is a downside to such solution, which would be that the driver then
>> needs call the detach API, just to do device_link_del(). Of course
>> then it would also needs to call the attach API later if/when needed.
>> Doing this adds unnecessary overhead - comparing to just let the
>> driver call device_link_add|del() when needed. On the upside, yes, it
>> would put less burden on the drivers as it then only needs to care
>> about using one set of functions.
>>
>> Which solution do you prefer?
> 
> Any reason why we could not add a 'boolean' argument to the API to indicate whether the new device should be linked? I think that I prefer the API handles it, but I can see there could be instances where drivers may wish to handle it themselves.
> 
> Rajendra, do you have a use-case right now where the driver would want to handle the linking?

So if I understand this right, any driver which does want to control individual powerdomain state would
need to do the linking itself right?

What I am saying is, if I have device A, with powerdomains X and Y, and if I want to turn on only X,
then I would want only X to be linked with A, and at a later point if I want both X and Y to be turned on,
I would then go ahead and link both X and Y to A? Is that correct or did I get it all wrong?

I know atleast Camera on msm8996 would need to do this since it has 2 vfe powerdoamins, which can be
turned on one at a time (depending on what resolution needs to be supported) or both together if we
really need very high resolution using both vfe modules. 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply

* [PATCH v2 2/4] drm/mediatek: Add support for mediatek SOC MT2712
From: Stu Hsieh @ 2018-05-23  9:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527053036.10410.18.camel@mtksdaap41>

On Wed, 2018-05-23 at 13:23 +0800, CK Hu wrote:
> Hi, Stu:
> 
> I've some inline comment.
> 
> On Wed, 2018-05-23 at 10:25 +0800, Stu Hsieh wrote:
> > This patch add support for the Mediatek MT2712 DISP subsystem.
> > There are two OVL engine and three disp output in MT2712.
> > 
> > Signed-off-by: Stu Hsieh <stu.hsieh@mediatek.com>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp.c      | 50 +++++++++++++++++++++++++++--
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |  8 +++--
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |  7 ++--
> >  drivers/gpu/drm/mediatek/mtk_drm_drv.c      | 47 +++++++++++++++++++++++++--
> >  drivers/gpu/drm/mediatek/mtk_drm_drv.h      |  7 ++--
> >  5 files changed, 108 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> > index 8130f3dab661..e563dedd1999 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> > @@ -29,6 +29,8 @@
> >  #define DISP_REG_CONFIG_DISP_COLOR0_SEL_IN	0x084
> >  #define DISP_REG_CONFIG_DISP_COLOR1_SEL_IN	0x088
> >  #define DISP_REG_CONFIG_DPI_SEL_IN		0x0ac
> > +#define DISP_REG_CONFIG_DISP_RDMA2_SOUT		0x0b8
> > +#define DISP_REG_CONFIG_DISP_RDMA0_MOUT_EN	0x0c4
> 
> These two definition are useless, so remove it.
ok


> 
> >  #define DISP_REG_CONFIG_DISP_RDMA1_MOUT_EN	0x0c8
> >  #define DISP_REG_CONFIG_MMSYS_CG_CON0		0x100
> >  
> > @@ -41,6 +43,7 @@
> >  #define DISP_REG_MUTEX_RST(n)	(0x28 + 0x20 * (n))
> >  #define DISP_REG_MUTEX_MOD(n)	(0x2c + 0x20 * (n))
> >  #define DISP_REG_MUTEX_SOF(n)	(0x30 + 0x20 * (n))
> > +#define DISP_REG_MUTEX_MOD2(n)	(0x34 + 0x20 * (n))
> 
> Move this to the patch 'drm/mediatek: support maximum 64 mutex mod' and
> that patch should be applied before this patch.
> 
> >  
> >  #define INT_MUTEX				BIT(1)
> >  
> > @@ -60,6 +63,25 @@
> >  #define MT8173_MUTEX_MOD_DISP_PWM1		BIT(24)
> >  #define MT8173_MUTEX_MOD_DISP_OD		BIT(25)
> >  
> > +#define MT2712_MUTEX_MOD_DISP_OVL0		BIT(11)
> > +#define MT2712_MUTEX_MOD_DISP_OVL1		BIT(12)
> > +#define MT2712_MUTEX_MOD_DISP_RDMA0		BIT(13)
> > +#define MT2712_MUTEX_MOD_DISP_RDMA1		BIT(14)
> > +#define MT2712_MUTEX_MOD_DISP_RDMA2		BIT(15)
> > +#define MT2712_MUTEX_MOD_DISP_WDMA0		BIT(16)
> > +#define MT2712_MUTEX_MOD_DISP_WDMA1		BIT(17)
> > +#define MT2712_MUTEX_MOD_DISP_COLOR0		BIT(18)
> > +#define MT2712_MUTEX_MOD_DISP_COLOR1		BIT(19)
> > +#define MT2712_MUTEX_MOD_DISP_AAL0		BIT(20)
> > +#define MT2712_MUTEX_MOD_DISP_UFOE		BIT(22)
> > +#define MT2712_MUTEX_MOD_DISP_PWM0		BIT(23)
> > +#define MT2712_MUTEX_MOD_DISP_PWM1		BIT(24)
> > +#define MT2712_MUTEX_MOD_DISP_PWM2		BIT(10)
> > +#define MT2712_MUTEX_MOD_DISP_OD0		BIT(25)
> > +/* modules more than 32, add BIT(31) when using DISP_REG_MUTEX_MOD2 bit */
> > +#define MT2712_MUTEX_MOD2_DISP_AAL1		(BIT(1) | BIT(31))
> 
> I think a better definition is
> 
> #define MT2712_MUTEX_MOD2_DISP_AAL1		BIT(33)
> 
> when you need to access this register,
> 
> if (ddp->mutex_mod[id] < BIT(32)) {
> 	offset = DISP_REG_MUTEX_MOD(mutex->id);
> 	reg = readl_relaxed(ddp->regs + offset);
> 	reg |= ddp->mutex_mod[id];
> 	writel_relaxed(reg, ddp->regs + offset);
> } else {
> 	offset = DISP_REG_MUTEX_MOD2(mutex->id);
> 	reg = readl_relaxed(ddp->regs + offset);
> 	reg |= (ddp->mutex_mod[id] >> 32);
> 	writel_relaxed(reg, ddp->regs + offset);
> }
> 
> because DISP_REG_MUTEX_MOD BIT(31) could be used for some module.

This modification is workable, but result some build warning like
following:
1. #define BIT(nr)   (1UL << (nr)) in include/linux/bitops.h
2. [DDP_COMPONENT_AAL1] = MT2712_MUTEX_MOD2_DISP_AAL1,
   => we need to modify the definition about "static const unsigned int
mt2712_mutex_mod"

> > +#define MT2712_MUTEX_MOD2_DISP_OD1		(BIT(2) | BIT(31))
> > +
> >  #define MT2701_MUTEX_MOD_DISP_OVL		BIT(3)
> >  #define MT2701_MUTEX_MOD_DISP_WDMA		BIT(6)
> >  #define MT2701_MUTEX_MOD_DISP_COLOR		BIT(7)
> > @@ -74,6 +96,7 @@
> >  
> >  #define OVL0_MOUT_EN_COLOR0		0x1
> >  #define OD_MOUT_EN_RDMA0		0x1
> > +#define OD1_MOUT_EN_RDMA1		BIT(16)
> >  #define UFOE_MOUT_EN_DSI0		0x1
> >  #define COLOR0_SEL_IN_OVL0		0x1
> >  #define OVL1_MOUT_EN_COLOR1		0x1
> > @@ -108,12 +131,32 @@ static const unsigned int mt2701_mutex_mod[DDP_COMPONENT_ID_MAX] = {
> >  	[DDP_COMPONENT_WDMA0] = MT2701_MUTEX_MOD_DISP_WDMA,
> >  };
> >  
> > +static const unsigned int mt2712_mutex_mod[DDP_COMPONENT_ID_MAX] = {
> > +	[DDP_COMPONENT_AAL0] = MT2712_MUTEX_MOD_DISP_AAL0,
> > +	[DDP_COMPONENT_AAL1] = MT2712_MUTEX_MOD2_DISP_AAL1,
> > +	[DDP_COMPONENT_COLOR0] = MT2712_MUTEX_MOD_DISP_COLOR0,
> > +	[DDP_COMPONENT_COLOR1] = MT2712_MUTEX_MOD_DISP_COLOR1,
> > +	[DDP_COMPONENT_OD0] = MT2712_MUTEX_MOD_DISP_OD0,
> > +	[DDP_COMPONENT_OD1] = MT2712_MUTEX_MOD2_DISP_OD1,
> > +	[DDP_COMPONENT_OVL0] = MT2712_MUTEX_MOD_DISP_OVL0,
> > +	[DDP_COMPONENT_OVL1] = MT2712_MUTEX_MOD_DISP_OVL1,
> > +	[DDP_COMPONENT_PWM0] = MT2712_MUTEX_MOD_DISP_PWM0,
> > +	[DDP_COMPONENT_PWM1] = MT2712_MUTEX_MOD_DISP_PWM1,
> > +	[DDP_COMPONENT_PWM2] = MT2712_MUTEX_MOD_DISP_PWM2,
> > +	[DDP_COMPONENT_RDMA0] = MT2712_MUTEX_MOD_DISP_RDMA0,
> > +	[DDP_COMPONENT_RDMA1] = MT2712_MUTEX_MOD_DISP_RDMA1,
> > +	[DDP_COMPONENT_RDMA2] = MT2712_MUTEX_MOD_DISP_RDMA2,
> > +	[DDP_COMPONENT_UFOE] = MT2712_MUTEX_MOD_DISP_UFOE,
> > +	[DDP_COMPONENT_WDMA0] = MT2712_MUTEX_MOD_DISP_WDMA0,
> > +	[DDP_COMPONENT_WDMA1] = MT2712_MUTEX_MOD_DISP_WDMA1,
> > +};
> > +
> >  static const unsigned int mt8173_mutex_mod[DDP_COMPONENT_ID_MAX] = {
> > -	[DDP_COMPONENT_AAL] = MT8173_MUTEX_MOD_DISP_AAL,
> > +	[DDP_COMPONENT_AAL0] = MT8173_MUTEX_MOD_DISP_AAL,
> >  	[DDP_COMPONENT_COLOR0] = MT8173_MUTEX_MOD_DISP_COLOR0,
> >  	[DDP_COMPONENT_COLOR1] = MT8173_MUTEX_MOD_DISP_COLOR1,
> >  	[DDP_COMPONENT_GAMMA] = MT8173_MUTEX_MOD_DISP_GAMMA,
> > -	[DDP_COMPONENT_OD] = MT8173_MUTEX_MOD_DISP_OD,
> > +	[DDP_COMPONENT_OD0] = MT8173_MUTEX_MOD_DISP_OD,
> >  	[DDP_COMPONENT_OVL0] = MT8173_MUTEX_MOD_DISP_OVL0,
> >  	[DDP_COMPONENT_OVL1] = MT8173_MUTEX_MOD_DISP_OVL1,
> >  	[DDP_COMPONENT_PWM0] = MT8173_MUTEX_MOD_DISP_PWM0,
> > @@ -138,7 +181,7 @@ static unsigned int mtk_ddp_mout_en(enum mtk_ddp_comp_id cur,
> >  	} else if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_RDMA0) {
> >  		*addr = DISP_REG_CONFIG_DISP_OVL_MOUT_EN;
> >  		value = OVL_MOUT_EN_RDMA;
> > -	} else if (cur == DDP_COMPONENT_OD && next == DDP_COMPONENT_RDMA0) {
> > +	} else if (cur == DDP_COMPONENT_OD0 && next == DDP_COMPONENT_RDMA0) {
> >  		*addr = DISP_REG_CONFIG_DISP_OD_MOUT_EN;
> >  		value = OD_MOUT_EN_RDMA0;
> >  	} else if (cur == DDP_COMPONENT_UFOE && next == DDP_COMPONENT_DSI0) {
> > @@ -407,6 +450,7 @@ static int mtk_ddp_remove(struct platform_device *pdev)
> >  
> >  static const struct of_device_id ddp_driver_dt_match[] = {
> >  	{ .compatible = "mediatek,mt2701-disp-mutex", .data = mt2701_mutex_mod},
> > +	{ .compatible = "mediatek,mt2712-disp-mutex", .data = mt2712_mutex_mod},
> >  	{ .compatible = "mediatek,mt8173-disp-mutex", .data = mt8173_mutex_mod},
> >  	{},
> >  };
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > index 4672317e3ad1..86e8c9e5df41 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > @@ -218,7 +218,8 @@ struct mtk_ddp_comp_match {
> >  };
> >  
> >  static const struct mtk_ddp_comp_match mtk_ddp_matches[DDP_COMPONENT_ID_MAX] = {
> > -	[DDP_COMPONENT_AAL]	= { MTK_DISP_AAL,	0, &ddp_aal },
> > +	[DDP_COMPONENT_AAL0]	= { MTK_DISP_AAL,	0, &ddp_aal },
> > +	[DDP_COMPONENT_AAL1]	= { MTK_DISP_AAL,	1, &ddp_aal },
> >  	[DDP_COMPONENT_BLS]	= { MTK_DISP_BLS,	0, NULL },
> >  	[DDP_COMPONENT_COLOR0]	= { MTK_DISP_COLOR,	0, NULL },
> >  	[DDP_COMPONENT_COLOR1]	= { MTK_DISP_COLOR,	1, NULL },
> > @@ -226,10 +227,13 @@ static const struct mtk_ddp_comp_match mtk_ddp_matches[DDP_COMPONENT_ID_MAX] = {
> >  	[DDP_COMPONENT_DSI0]	= { MTK_DSI,		0, NULL },
> >  	[DDP_COMPONENT_DSI1]	= { MTK_DSI,		1, NULL },
> >  	[DDP_COMPONENT_GAMMA]	= { MTK_DISP_GAMMA,	0, &ddp_gamma },
> > -	[DDP_COMPONENT_OD]	= { MTK_DISP_OD,	0, &ddp_od },
> > +	[DDP_COMPONENT_OD0]	= { MTK_DISP_OD,	0, &ddp_od },
> > +	[DDP_COMPONENT_OD1]	= { MTK_DISP_OD,	1, &ddp_od },
> >  	[DDP_COMPONENT_OVL0]	= { MTK_DISP_OVL,	0, NULL },
> >  	[DDP_COMPONENT_OVL1]	= { MTK_DISP_OVL,	1, NULL },
> >  	[DDP_COMPONENT_PWM0]	= { MTK_DISP_PWM,	0, NULL },
> > +	[DDP_COMPONENT_PWM1]	= { MTK_DISP_PWM,	1, NULL },
> > +	[DDP_COMPONENT_PWM2]	= { MTK_DISP_PWM,	2, NULL },
> >  	[DDP_COMPONENT_RDMA0]	= { MTK_DISP_RDMA,	0, NULL },
> >  	[DDP_COMPONENT_RDMA1]	= { MTK_DISP_RDMA,	1, NULL },
> >  	[DDP_COMPONENT_RDMA2]	= { MTK_DISP_RDMA,	2, NULL },
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > index 0828cf8bf85c..e00c2e798abd 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > @@ -41,7 +41,8 @@ enum mtk_ddp_comp_type {
> >  };
> >  
> >  enum mtk_ddp_comp_id {
> > -	DDP_COMPONENT_AAL,
> > +	DDP_COMPONENT_AAL0,
> > +	DDP_COMPONENT_AAL1,
> 
> Move this to a patch 'add ddp component AAL1'.
ok

> 
> >  	DDP_COMPONENT_BLS,
> >  	DDP_COMPONENT_COLOR0,
> >  	DDP_COMPONENT_COLOR1,
> > @@ -49,11 +50,13 @@ enum mtk_ddp_comp_id {
> >  	DDP_COMPONENT_DSI0,
> >  	DDP_COMPONENT_DSI1,
> >  	DDP_COMPONENT_GAMMA,
> > -	DDP_COMPONENT_OD,
> > +	DDP_COMPONENT_OD0,
> > +	DDP_COMPONENT_OD1,
> 
> Move this to a patch 'add ddp component OD1'.
ok

> 
> >  	DDP_COMPONENT_OVL0,
> >  	DDP_COMPONENT_OVL1,
> >  	DDP_COMPONENT_PWM0,
> >  	DDP_COMPONENT_PWM1,
> > +	DDP_COMPONENT_PWM2,
> 
> Move this to a patch 'add ddp component PWM2'.
ok

> 
> >  	DDP_COMPONENT_RDMA0,
> >  	DDP_COMPONENT_RDMA1,
> >  	DDP_COMPONENT_RDMA2,
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > index a2ca90fc403c..3a866e1d6af4 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > @@ -146,11 +146,37 @@ static const enum mtk_ddp_comp_id mt2701_mtk_ddp_ext[] = {
> >  	DDP_COMPONENT_DPI0,
> >  };
> >  
> > +static const enum mtk_ddp_comp_id mt2712_mtk_ddp_main[] = {
> > +	DDP_COMPONENT_OVL0,
> > +	DDP_COMPONENT_COLOR0,
> > +	DDP_COMPONENT_AAL0,
> > +	DDP_COMPONENT_OD0,
> > +	DDP_COMPONENT_RDMA0,
> > +	DDP_COMPONENT_DPI0,
> > +	DDP_COMPONENT_PWM0,
> > +};
> > +
> > +static const enum mtk_ddp_comp_id mt2712_mtk_ddp_ext[] = {
> > +	DDP_COMPONENT_OVL1,
> > +	DDP_COMPONENT_COLOR1,
> > +	DDP_COMPONENT_AAL1,
> > +	DDP_COMPONENT_OD1,
> > +	DDP_COMPONENT_RDMA1,
> > +	DDP_COMPONENT_DPI1,
> > +	DDP_COMPONENT_PWM1,
> > +};
> > +
> > +static const enum mtk_ddp_comp_id mt2712_mtk_ddp_third[] = {
> > +	DDP_COMPONENT_RDMA2,
> > +	DDP_COMPONENT_DSI2,
> > +	DDP_COMPONENT_PWM2,
> > +};
> > +
> >  static const enum mtk_ddp_comp_id mt8173_mtk_ddp_main[] = {
> >  	DDP_COMPONENT_OVL0,
> >  	DDP_COMPONENT_COLOR0,
> > -	DDP_COMPONENT_AAL,
> > -	DDP_COMPONENT_OD,
> > +	DDP_COMPONENT_AAL0,
> > +	DDP_COMPONENT_OD0,
> >  	DDP_COMPONENT_RDMA0,
> >  	DDP_COMPONENT_UFOE,
> >  	DDP_COMPONENT_DSI0,
> > @@ -173,6 +199,15 @@ static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = {
> >  	.shadow_register = true,
> >  };
> >  
> > +static const struct mtk_mmsys_driver_data mt2712_mmsys_driver_data = {
> > +	.main_path = mt2712_mtk_ddp_main,
> > +	.main_len = ARRAY_SIZE(mt2712_mtk_ddp_main),
> > +	.ext_path = mt2712_mtk_ddp_ext,
> > +	.ext_len = ARRAY_SIZE(mt2712_mtk_ddp_ext),
> > +	.third_path = mt2712_mtk_ddp_third,
> > +	.third_len = ARRAY_SIZE(mt2712_mtk_ddp_third),
> > +};
> > +
> >  static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
> >  	.main_path = mt8173_mtk_ddp_main,
> >  	.main_len = ARRAY_SIZE(mt8173_mtk_ddp_main),
> > @@ -232,6 +267,11 @@ static int mtk_drm_kms_init(struct drm_device *drm)
> >  	if (ret < 0)
> >  		goto err_component_unbind;
> >  
> > +	ret = mtk_drm_crtc_create(drm, private->data->third_path,
> > +				  private->data->third_len);
> > +	if (ret < 0)
> > +		goto err_component_unbind;
> > +
> 
> Move this to a patch 'add third ddp path'.
ok

> 
> Regards,
> CK
> 
> >  	/* Use OVL device for all DMA memory allocations */
> >  	np = private->comp_node[private->data->main_path[0]] ?:
> >  	     private->comp_node[private->data->ext_path[0]];
> > @@ -374,6 +414,7 @@ static const struct of_device_id mtk_ddp_comp_dt_ids[] = {
> >  	{ .compatible = "mediatek,mt8173-dsi",        .data = (void *)MTK_DSI },
> >  	{ .compatible = "mediatek,mt8173-dpi",        .data = (void *)MTK_DPI },
> >  	{ .compatible = "mediatek,mt2701-disp-mutex", .data = (void *)MTK_DISP_MUTEX },
> > +	{ .compatible = "mediatek,mt2712-disp-mutex", .data = (void *)MTK_DISP_MUTEX },
> >  	{ .compatible = "mediatek,mt8173-disp-mutex", .data = (void *)MTK_DISP_MUTEX },
> >  	{ .compatible = "mediatek,mt2701-disp-pwm",   .data = (void *)MTK_DISP_BLS },
> >  	{ .compatible = "mediatek,mt8173-disp-pwm",   .data = (void *)MTK_DISP_PWM },
> > @@ -552,6 +593,8 @@ static SIMPLE_DEV_PM_OPS(mtk_drm_pm_ops, mtk_drm_sys_suspend,
> >  static const struct of_device_id mtk_drm_of_ids[] = {
> >  	{ .compatible = "mediatek,mt2701-mmsys",
> >  	  .data = &mt2701_mmsys_driver_data},
> > +	{ .compatible = "mediatek,mt2712-mmsys",
> > +	  .data = &mt2712_mmsys_driver_data},
> >  	{ .compatible = "mediatek,mt8173-mmsys",
> >  	  .data = &mt8173_mmsys_driver_data},
> >  	{ }
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > index c3378c452c0a..e821342bc2d3 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > @@ -17,8 +17,8 @@
> >  #include <linux/io.h>
> >  #include "mtk_drm_ddp_comp.h"
> >  
> > -#define MAX_CRTC	2
> > -#define MAX_CONNECTOR	2
> > +#define MAX_CRTC	3
> > +#define MAX_CONNECTOR	3
> >  
> >  struct device;
> >  struct device_node;
> > @@ -33,6 +33,9 @@ struct mtk_mmsys_driver_data {
> >  	unsigned int main_len;
> >  	const enum mtk_ddp_comp_id *ext_path;
> >  	unsigned int ext_len;
> > +	enum mtk_ddp_comp_id *third_path;
> > +	unsigned int third_len;
> > +
> >  	bool shadow_register;
> >  };
> >  
> 
> 

^ permalink raw reply

* [PATCH] ARM: dts: stm32: Add DFSDM support to stm32mp157c
From: Fabrice Gasnier @ 2018-05-23  9:30 UTC (permalink / raw)
  To: linux-arm-kernel

stm32mp157c has DFSDM (Digital Filter For Sigma Delta Modulators) hardware
with 6 filter instances.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 arch/arm/boot/dts/stm32mp157c.dtsi | 70 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
index 66d7496..3d2f4e7 100644
--- a/arch/arm/boot/dts/stm32mp157c.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c.dtsi
@@ -556,6 +556,76 @@
 			};
 		};
 
+		dfsdm: dfsdm at 4400d000 {
+			compatible = "st,stm32mp1-dfsdm";
+			reg = <0x4400d000 0x800>;
+			clocks = <&rcc DFSDM_K>;
+			clock-names = "dfsdm";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+
+			dfsdm0: filter at 0 {
+				compatible = "st,stm32-dfsdm-adc";
+				#io-channel-cells = <1>;
+				reg = <0>;
+				interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dmamux1 101 0x400 0x01>;
+				dma-names = "rx";
+				status = "disabled";
+			};
+
+			dfsdm1: filter at 1 {
+				compatible = "st,stm32-dfsdm-adc";
+				#io-channel-cells = <1>;
+				reg = <1>;
+				interrupts = <GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dmamux1 102 0x400 0x01>;
+				dma-names = "rx";
+				status = "disabled";
+			};
+
+			dfsdm2: filter at 2 {
+				compatible = "st,stm32-dfsdm-adc";
+				#io-channel-cells = <1>;
+				reg = <2>;
+				interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dmamux1 103 0x400 0x01>;
+				dma-names = "rx";
+				status = "disabled";
+			};
+
+			dfsdm3: filter at 3 {
+				compatible = "st,stm32-dfsdm-adc";
+				#io-channel-cells = <1>;
+				reg = <3>;
+				interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dmamux1 104 0x400 0x01>;
+				dma-names = "rx";
+				status = "disabled";
+			};
+
+			dfsdm4: filter at 4 {
+				compatible = "st,stm32-dfsdm-adc";
+				#io-channel-cells = <1>;
+				reg = <4>;
+				interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dmamux1 91 0x400 0x01>;
+				dma-names = "rx";
+				status = "disabled";
+			};
+
+			dfsdm5: filter at 5 {
+				compatible = "st,stm32-dfsdm-adc";
+				#io-channel-cells = <1>;
+				reg = <5>;
+				interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>;
+				dmas = <&dmamux1 92 0x400 0x01>;
+				dma-names = "rx";
+				status = "disabled";
+			};
+		};
+
 		dma1: dma at 48000000 {
 			compatible = "st,stm32-dma";
 			reg = <0x48000000 0x400>;
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 4/4] drm/mediatek: add connection from OD1 to RDMA1
From: Stu Hsieh @ 2018-05-23  9:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527055278.21354.3.camel@mtksdaap41>

Hi, CK:

On Wed, 2018-05-23 at 14:01 +0800, CK Hu wrote:
> Hi, Stu:
> 
> On Wed, 2018-05-23 at 10:25 +0800, Stu Hsieh wrote:
> > This patch add the connection from OD1 to RDMA1 for ext path.
> > 
> 
> I would like to apply this patch before the patch 'Add support for
> mediatek SOC MT2712' because this patch is necessary for mt2712.
ok

> 
> Regards,
> CK
> 
> > Signed-off-by: Stu Hsieh <stu.hsieh@mediatek.com>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> > index 3c7bd453cf42..0450ecbbc356 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> > @@ -193,6 +193,9 @@ static unsigned int mtk_ddp_mout_en(enum mtk_ddp_comp_id cur,
> >  	} else if (cur == DDP_COMPONENT_GAMMA && next == DDP_COMPONENT_RDMA1) {
> >  		*addr = DISP_REG_CONFIG_DISP_GAMMA_MOUT_EN;
> >  		value = GAMMA_MOUT_EN_RDMA1;
> > +	} else if (cur == DDP_COMPONENT_OD1 && next == DDP_COMPONENT_RDMA1) {
> > +		*addr = DISP_REG_CONFIG_DISP_OD_MOUT_EN;
> > +		value = OD1_MOUT_EN_RDMA1;
> >  	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI0) {
> >  		*addr = DISP_REG_CONFIG_DISP_RDMA1_MOUT_EN;
> >  		value = RDMA1_MOUT_DPI0;
> 
> 

^ permalink raw reply

* [PATCH v5 0/7] Add tda998x (HDMI) support to atmel-hlcdc
From: Peter Rosin @ 2018-05-23  9:31 UTC (permalink / raw)
  To: linux-arm-kernel

Hi!

I naively thought that since there was support for both nxp,tda19988 (in
the tda998x driver) and the atmel-hlcdc, things would be a smooth ride.
But it wasn't, so I started looking around and realized I had to fix
things.

In v1 and v2 I fixed things by making the atmel-hlcdc driver a master
component, but now I fix things by making the tda998x driver a bridge
instead. This was after a suggestion from Boris Brezillon the
atmel-hlcdc maintainer), so there was some risk of bias ... but after
comparing what was needed, I too find the bridge approach more direct.
In v4, an issue was noted by Russell King in that the tda998x device
might be unbound with shattering results for the main drm device.
This is of course a real problem and it needs to be fixed. I have
submitted a series [1] that tries to do that. However, since there are
currently *10* other bridges (analogix-anx78xx, adv7511, megachips_stdp*,
nxp-ptn3460, parade-ps8622, sii902x, sii9234, sil-sii8620, tc358767 and
ti-tfp410) with exactly the same problem (they are all I2C devices that
might be unexpectedly unbound without safety-net, at least that is how I
read it), I think it should be ok to add one more and then fix them all
instead of holding this series hostage. Apparently people don't go around
and unbind I2C devices all that often...

In addition to the above, our PCB interface between the SAMA5D3 and the
HDMI encoder is only using 16 bits, and this has to be described
somewhere, or the atmel-hlcdc driver have no chance of selecting the
correct output mode. Since I have similar problems with a ds90c185 lvds
encoder I added patches to override the atmel-hlcdc output format via
DT properties compatible with the media video-interface binding and
things start to play together.

Anyway, this series solves some real issues for my HW.

Also, is it perhaps possible that patches 1-3 can be taken independently
from 4-7? There is no hard dependency between the two parts of this series.
Patches 1-3 have the relevant tags and should be uncontroversial...

Cheers,
Peter

Changes since v4   https://lkml.org/lkml/2018/4/23/92
- added reviewed-by from Rob to patch 2/7
- dropped patch 8, since the issue noted by Russell King is not present
  when working with components as tilcdc currently do when handling tda998x

Changes since v3   https://lkml.org/lkml/2018/4/19/736
- moved the meat of the drm_of_media_bus_fmt function from patch 3/7
  to a driver-local atmel_hlcdc_of_bus_fmt function, and squashed with
  patch 4/7 (this is now patch 3/8).
- patch 3/8 now parses and stores the DT bus format together with the
  encoder in a reintroduced struct atmel_hlcdc_rgb_output instead of
  allocating a separate encoder-indexed array for the bus formats.
- patch 5/8 is new and splits tda988x_encoder_dpms into a pair of
  functions to enable/disable separately. This fits better with both
  the current code and for the followup drm_bridge patch (7/8).
- adjust patch 6/8 and 7/8 to the above simplification.
- added patch 8/8 that removes component master code from tilcdc.

Changes since v2   https://lkml.org/lkml/2018/4/17/385
- patch 2/7 fixed spelling and added an example
- patch 4/7 parse the DT up front and store the result indexed by encoder
- old patch 5/6 and 6/6 dropped
- patch 5-7/7 are new and makes the tda998x driver a drm_bridge

Changes since v1   https://lkml.org/lkml/2018/4/9/294
- added reviewed-by from Rob to patch 1/6
- patch 2/6 changed so that the bus format override is in the endpoint
  DT node, and follows the binding of media video-interfaces.
- patch 3/6 is new, it adds drm_of_media_bus_fmt which parses above
  media video-interface binding (partially).
- patch 4/6 now makes use of the above helper (and also fixes problems
  with the 3/5 patch from v1 when no override was specified).
- do not mention unrelated connector display_info details in the cover
  letter and commit messages.

[1] https://lkml.org/lkml/2018/5/16/380

Peter Rosin (7):
  dt-bindings: display: bridge: lvds-transmitter: add ti,ds90c185
  dt-bindings: display: atmel: optional video-interface of endpoints
  drm/atmel-hlcdc: support bus-width (12/16/18/24) in endpoint nodes
  drm/i2c: tda998x: find the drm_device via the drm_connector
  drm/i2c: tda998x: split tda998x_encoder_dpms into enable/disable
  drm/i2c: tda998x: split encoder and component functions from the work
  drm/i2c: tda998x: register as a drm bridge

 .../devicetree/bindings/display/atmel/hlcdc-dc.txt |  26 ++
 .../bindings/display/bridge/lvds-transmitter.txt   |   8 +-
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c     |  70 ++++--
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h       |   1 +
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  67 +++++-
 drivers/gpu/drm/i2c/tda998x_drv.c                  | 268 ++++++++++++++++-----
 6 files changed, 351 insertions(+), 89 deletions(-)

-- 
2.11.0

^ permalink raw reply

* [PATCH v5 1/7] dt-bindings: display: bridge: lvds-transmitter: add ti, ds90c185
From: Peter Rosin @ 2018-05-23  9:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180523093122.27859-1-peda@axentia.se>

Start list of actual chips compatible with "lvds-encoder".

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../devicetree/bindings/display/bridge/lvds-transmitter.txt       | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
index fd39ad34c383..50220190c203 100644
--- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
+++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
@@ -22,7 +22,13 @@ among others.
 
 Required properties:
 
-- compatible: Must be "lvds-encoder"
+- compatible: Must be one or more of the following
+  - "ti,ds90c185" for the TI DS90C185 FPD-Link Serializer
+  - "lvds-encoder" for a generic LVDS encoder device
+
+  When compatible with the generic version, nodes must list the
+  device-specific version corresponding to the device first
+  followed by the generic version.
 
 Required nodes:
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH v5 2/7] dt-bindings: display: atmel: optional video-interface of endpoints
From: Peter Rosin @ 2018-05-23  9:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180523093122.27859-1-peda@axentia.se>

With bus-type/bus-width properties in the endpoint nodes, the video-
interface of the connection can be specified for cases where the
heuristic fails to select the correct output mode. This can happen
e.g. if not all RGB pins are routed on the PCB; the driver has no
way of knowing this, and needs to be told explicitly.

This is critical for the devices that have the "conflicting output
formats" issue (SAM9N12, SAM9X5, SAMA5D3), since the most significant
RGB bits move around depending on the selected output mode. For
devices that do not have the "conflicting output formats" issue
(SAMA5D2, SAMA5D4), this is completely irrelevant.

Acked-by: Boris Brezillon <boris.brezillon@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../devicetree/bindings/display/atmel/hlcdc-dc.txt | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt b/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
index 82f2acb3d374..9de434a8f523 100644
--- a/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
+++ b/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
@@ -15,6 +15,14 @@ Required children nodes:
  to external devices using the OF graph reprensentation (see ../graph.txt).
  At least one port node is required.
 
+Optional properties in grandchild nodes:
+ Any endpoint grandchild node may specify a desired video interface
+ according to ../../media/video-interfaces.txt, specifically
+ - bus-type: must be <0>.
+ - bus-width: recognized values are <12>, <16>, <18> and <24>, and
+   override any output mode selection heuristic, forcing "rgb444",
+   "rgb565", "rgb666" and "rgb888" respectively.
+
 Example:
 
 	hlcdc: hlcdc at f0030000 {
@@ -50,3 +58,21 @@ Example:
 			#pwm-cells = <3>;
 		};
 	};
+
+
+Example 2: With a video interface override to force rgb565; as above
+but with these changes/additions:
+
+	&hlcdc {
+		hlcdc-display-controller {
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb565>;
+
+			port at 0 {
+				hlcdc_panel_output: endpoint at 0 {
+					bus-type = <0>;
+					bus-width = <16>;
+				};
+			};
+		};
+	};
-- 
2.11.0

^ permalink raw reply related

* [PATCH v5 3/7] drm/atmel-hlcdc: support bus-width (12/16/18/24) in endpoint nodes
From: Peter Rosin @ 2018-05-23  9:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180523093122.27859-1-peda@axentia.se>

This beats the heuristic that the connector is involved in what format
should be output for cases where this fails.

E.g. if there is a bridge that changes format between the encoder and the
connector, or if some of the RGB pins between the lcd controller and the
encoder are not routed on the PCB.

This is critical for the devices that have the "conflicting output
formats" issue (SAM9N12, SAM9X5, SAMA5D3), since the most significant
RGB bits move around depending on the selected output mode. For
devices that do not have the "conflicting output formats" issue
(SAMA5D2, SAMA5D4), this is completely irrelevant.

Acked-by: Boris Brezillon <boris.brezillon@bootlin.com>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c   | 70 +++++++++++++++++-------
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h     |  1 +
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 67 ++++++++++++++++++++---
 3 files changed, 111 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index d73281095fac..c38a479ada98 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -226,6 +226,55 @@ static void atmel_hlcdc_crtc_atomic_enable(struct drm_crtc *c,
 #define ATMEL_HLCDC_RGB888_OUTPUT	BIT(3)
 #define ATMEL_HLCDC_OUTPUT_MODE_MASK	GENMASK(3, 0)
 
+static int atmel_hlcdc_connector_output_mode(struct drm_connector_state *state)
+{
+	struct drm_connector *connector = state->connector;
+	struct drm_display_info *info = &connector->display_info;
+	struct drm_encoder *encoder;
+	unsigned int supported_fmts = 0;
+	int j;
+
+	encoder = state->best_encoder;
+	if (!encoder)
+		encoder = connector->encoder;
+
+	switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
+	case 0:
+		break;
+	case MEDIA_BUS_FMT_RGB444_1X12:
+		return ATMEL_HLCDC_RGB444_OUTPUT;
+	case MEDIA_BUS_FMT_RGB565_1X16:
+		return ATMEL_HLCDC_RGB565_OUTPUT;
+	case MEDIA_BUS_FMT_RGB666_1X18:
+		return ATMEL_HLCDC_RGB666_OUTPUT;
+	case MEDIA_BUS_FMT_RGB888_1X24:
+		return ATMEL_HLCDC_RGB888_OUTPUT;
+	default:
+		return -EINVAL;
+	}
+
+	for (j = 0; j < info->num_bus_formats; j++) {
+		switch (info->bus_formats[j]) {
+		case MEDIA_BUS_FMT_RGB444_1X12:
+			supported_fmts |= ATMEL_HLCDC_RGB444_OUTPUT;
+			break;
+		case MEDIA_BUS_FMT_RGB565_1X16:
+			supported_fmts |= ATMEL_HLCDC_RGB565_OUTPUT;
+			break;
+		case MEDIA_BUS_FMT_RGB666_1X18:
+			supported_fmts |= ATMEL_HLCDC_RGB666_OUTPUT;
+			break;
+		case MEDIA_BUS_FMT_RGB888_1X24:
+			supported_fmts |= ATMEL_HLCDC_RGB888_OUTPUT;
+			break;
+		default:
+			break;
+		}
+	}
+
+	return supported_fmts;
+}
+
 static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
 {
 	unsigned int output_fmts = ATMEL_HLCDC_OUTPUT_MODE_MASK;
@@ -238,31 +287,12 @@ static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
 	crtc = drm_crtc_to_atmel_hlcdc_crtc(state->crtc);
 
 	for_each_new_connector_in_state(state->state, connector, cstate, i) {
-		struct drm_display_info *info = &connector->display_info;
 		unsigned int supported_fmts = 0;
-		int j;
 
 		if (!cstate->crtc)
 			continue;
 
-		for (j = 0; j < info->num_bus_formats; j++) {
-			switch (info->bus_formats[j]) {
-			case MEDIA_BUS_FMT_RGB444_1X12:
-				supported_fmts |= ATMEL_HLCDC_RGB444_OUTPUT;
-				break;
-			case MEDIA_BUS_FMT_RGB565_1X16:
-				supported_fmts |= ATMEL_HLCDC_RGB565_OUTPUT;
-				break;
-			case MEDIA_BUS_FMT_RGB666_1X18:
-				supported_fmts |= ATMEL_HLCDC_RGB666_OUTPUT;
-				break;
-			case MEDIA_BUS_FMT_RGB888_1X24:
-				supported_fmts |= ATMEL_HLCDC_RGB888_OUTPUT;
-				break;
-			default:
-				break;
-			}
-		}
+		supported_fmts = atmel_hlcdc_connector_output_mode(cstate);
 
 		if (crtc->dc->desc->conflicting_output_formats)
 			output_fmts &= supported_fmts;
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
index 60c937f42114..4cc1e03f0aee 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
@@ -441,5 +441,6 @@ void atmel_hlcdc_crtc_irq(struct drm_crtc *c);
 int atmel_hlcdc_crtc_create(struct drm_device *dev);
 
 int atmel_hlcdc_create_outputs(struct drm_device *dev);
+int atmel_hlcdc_encoder_get_bus_fmt(struct drm_encoder *encoder);
 
 #endif /* DRM_ATMEL_HLCDC_H */
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
index 8db51fb131db..db4d6aaaef93 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
@@ -27,33 +27,86 @@
 
 #include "atmel_hlcdc_dc.h"
 
+struct atmel_hlcdc_rgb_output {
+	struct drm_encoder encoder;
+	int bus_fmt;
+};
+
 static const struct drm_encoder_funcs atmel_hlcdc_panel_encoder_funcs = {
 	.destroy = drm_encoder_cleanup,
 };
 
+static struct atmel_hlcdc_rgb_output *
+atmel_hlcdc_encoder_to_rgb_output(struct drm_encoder *encoder)
+{
+	return container_of(encoder, struct atmel_hlcdc_rgb_output, encoder);
+}
+
+int atmel_hlcdc_encoder_get_bus_fmt(struct drm_encoder *encoder)
+{
+	struct atmel_hlcdc_rgb_output *output;
+
+	output = atmel_hlcdc_encoder_to_rgb_output(encoder);
+
+	return output->bus_fmt;
+}
+
+static int atmel_hlcdc_of_bus_fmt(struct device_node *ep)
+{
+	u32 bus_width = 0;
+
+	of_property_read_u32(ep, "bus-width", &bus_width);
+
+	switch (bus_width) {
+	case 0:
+		return 0;
+	case 12:
+		return MEDIA_BUS_FMT_RGB444_1X12;
+	case 16:
+		return MEDIA_BUS_FMT_RGB565_1X16;
+	case 18:
+		return MEDIA_BUS_FMT_RGB666_1X18;
+	case 24:
+		return MEDIA_BUS_FMT_RGB888_1X24;
+	default:
+		return -EINVAL;
+	}
+}
+
 static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, int endpoint)
 {
-	struct drm_encoder *encoder;
+	struct atmel_hlcdc_rgb_output *output;
+	struct device_node *ep;
 	struct drm_panel *panel;
 	struct drm_bridge *bridge;
 	int ret;
 
+	ep = of_graph_get_endpoint_by_regs(dev->dev->of_node, 0, endpoint);
+	if (!ep)
+		return -ENODEV;
+
 	ret = drm_of_find_panel_or_bridge(dev->dev->of_node, 0, endpoint,
 					  &panel, &bridge);
 	if (ret)
 		return ret;
 
-	encoder = devm_kzalloc(dev->dev, sizeof(*encoder), GFP_KERNEL);
-	if (!encoder)
+	output = devm_kzalloc(dev->dev, sizeof(*output), GFP_KERNEL);
+	if (!output)
 		return -EINVAL;
 
-	ret = drm_encoder_init(dev, encoder,
+	output->bus_fmt = atmel_hlcdc_of_bus_fmt(ep);
+	if (output->bus_fmt < 0) {
+		dev_err(dev->dev, "invalid bus width\n");
+		return -EINVAL;
+	}
+
+	ret = drm_encoder_init(dev, &output->encoder,
 			       &atmel_hlcdc_panel_encoder_funcs,
 			       DRM_MODE_ENCODER_NONE, NULL);
 	if (ret)
 		return ret;
 
-	encoder->possible_crtcs = 0x1;
+	output->encoder.possible_crtcs = 0x1;
 
 	if (panel) {
 		bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_Unknown);
@@ -62,7 +115,7 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, int endpoint)
 	}
 
 	if (bridge) {
-		ret = drm_bridge_attach(encoder, bridge, NULL);
+		ret = drm_bridge_attach(&output->encoder, bridge, NULL);
 		if (!ret)
 			return 0;
 
@@ -70,7 +123,7 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, int endpoint)
 			drm_panel_bridge_remove(bridge);
 	}
 
-	drm_encoder_cleanup(encoder);
+	drm_encoder_cleanup(&output->encoder);
 
 	return ret;
 }
-- 
2.11.0

^ permalink raw reply related

* [PATCH v5 4/7] drm/i2c: tda998x: find the drm_device via the drm_connector
From: Peter Rosin @ 2018-05-23  9:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180523093122.27859-1-peda@axentia.se>

This prepares for being a drm_bridge which will not register the
encoder. That makes the connector the better choice.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 421c8a72369e..933d309d2e0f 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -630,7 +630,7 @@ static void tda998x_detect_work(struct work_struct *work)
 {
 	struct tda998x_priv *priv =
 		container_of(work, struct tda998x_priv, detect_work);
-	struct drm_device *dev = priv->encoder.dev;
+	struct drm_device *dev = priv->connector.dev;
 
 	if (dev)
 		drm_kms_helper_hotplug_event(dev);
-- 
2.11.0

^ permalink raw reply related

* [PATCH v5 5/7] drm/i2c: tda998x: split tda998x_encoder_dpms into enable/disable
From: Peter Rosin @ 2018-05-23  9:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180523093122.27859-1-peda@axentia.se>

This fits better with the drm_bridge callbacks for when this
driver becomes a drm_bridge.

Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 64 ++++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 933d309d2e0f..3dda07a2fd2f 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1163,36 +1163,42 @@ static int tda998x_connector_init(struct tda998x_priv *priv,
 
 /* DRM encoder functions */
 
-static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
+static void tda998x_enable(struct tda998x_priv *priv)
 {
-	struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
-	bool on;
+	if (priv->is_on)
+		return;
 
-	/* we only care about on or off: */
-	on = mode == DRM_MODE_DPMS_ON;
+	/* enable video ports, audio will be enabled later */
+	reg_write(priv, REG_ENA_VP_0, 0xff);
+	reg_write(priv, REG_ENA_VP_1, 0xff);
+	reg_write(priv, REG_ENA_VP_2, 0xff);
+	/* set muxing after enabling ports: */
+	reg_write(priv, REG_VIP_CNTRL_0, priv->vip_cntrl_0);
+	reg_write(priv, REG_VIP_CNTRL_1, priv->vip_cntrl_1);
+	reg_write(priv, REG_VIP_CNTRL_2, priv->vip_cntrl_2);
 
-	if (on == priv->is_on)
-		return;
+	priv->is_on = true;
+}
 
-	if (on) {
-		/* enable video ports, audio will be enabled later */
-		reg_write(priv, REG_ENA_VP_0, 0xff);
-		reg_write(priv, REG_ENA_VP_1, 0xff);
-		reg_write(priv, REG_ENA_VP_2, 0xff);
-		/* set muxing after enabling ports: */
-		reg_write(priv, REG_VIP_CNTRL_0, priv->vip_cntrl_0);
-		reg_write(priv, REG_VIP_CNTRL_1, priv->vip_cntrl_1);
-		reg_write(priv, REG_VIP_CNTRL_2, priv->vip_cntrl_2);
-
-		priv->is_on = true;
-	} else {
-		/* disable video ports */
-		reg_write(priv, REG_ENA_VP_0, 0x00);
-		reg_write(priv, REG_ENA_VP_1, 0x00);
-		reg_write(priv, REG_ENA_VP_2, 0x00);
+static void tda998x_disable(struct tda998x_priv *priv)
+{
+	/* disable video ports */
+	reg_write(priv, REG_ENA_VP_0, 0x00);
+	reg_write(priv, REG_ENA_VP_1, 0x00);
+	reg_write(priv, REG_ENA_VP_2, 0x00);
 
-		priv->is_on = false;
-	}
+	priv->is_on = false;
+}
+
+static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
+{
+	struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
+
+	/* we only care about on or off: */
+	if (mode == DRM_MODE_DPMS_ON)
+		tda998x_enable(priv);
+	else
+		tda998x_disable(priv);
 }
 
 static void
@@ -1606,12 +1612,16 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
 
 static void tda998x_encoder_prepare(struct drm_encoder *encoder)
 {
-	tda998x_encoder_dpms(encoder, DRM_MODE_DPMS_OFF);
+	struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
+
+	tda998x_disable(priv);
 }
 
 static void tda998x_encoder_commit(struct drm_encoder *encoder)
 {
-	tda998x_encoder_dpms(encoder, DRM_MODE_DPMS_ON);
+	struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
+
+	tda998x_enable(priv);
 }
 
 static const struct drm_encoder_helper_funcs tda998x_encoder_helper_funcs = {
-- 
2.11.0

^ permalink raw reply related

* [PATCH v5 6/7] drm/i2c: tda998x: split encoder and component functions from the work
From: Peter Rosin @ 2018-05-23  9:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180523093122.27859-1-peda@axentia.se>

This enables reuse of the machinery for the case where a drm_bridge
needs to do the same work via different interfaces.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 3dda07a2fd2f..d401b3d0095c 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1202,11 +1202,10 @@ static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
 }
 
 static void
-tda998x_encoder_mode_set(struct drm_encoder *encoder,
-			 struct drm_display_mode *mode,
-			 struct drm_display_mode *adjusted_mode)
+tda998x_mode_set(struct tda998x_priv *priv,
+		 struct drm_display_mode *mode,
+		 struct drm_display_mode *adjusted_mode)
 {
-	struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
 	u16 ref_pix, ref_line, n_pix, n_line;
 	u16 hs_pix_s, hs_pix_e;
 	u16 vs1_pix_s, vs1_pix_e, vs1_line_s, vs1_line_e;
@@ -1413,6 +1412,16 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
 	mutex_unlock(&priv->audio_mutex);
 }
 
+static void
+tda998x_encoder_mode_set(struct drm_encoder *encoder,
+			 struct drm_display_mode *mode,
+			 struct drm_display_mode *adjusted_mode)
+{
+	struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
+
+	tda998x_mode_set(priv, mode, adjusted_mode);
+}
+
 static void tda998x_destroy(struct tda998x_priv *priv)
 {
 	/* disable all IRQs and free the IRQ handler */
@@ -1662,11 +1671,10 @@ static void tda998x_set_config(struct tda998x_priv *priv,
 	priv->audio_params = p->audio_params;
 }
 
-static int tda998x_bind(struct device *dev, struct device *master, void *data)
+static int tda998x_init(struct device *dev, struct drm_device *drm)
 {
 	struct tda998x_encoder_params *params = dev->platform_data;
 	struct i2c_client *client = to_i2c_client(dev);
-	struct drm_device *drm = data;
 	struct tda998x_priv *priv;
 	u32 crtcs = 0;
 	int ret;
@@ -1714,8 +1722,7 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
 	return ret;
 }
 
-static void tda998x_unbind(struct device *dev, struct device *master,
-			   void *data)
+static void tda998x_fini(struct device *dev)
 {
 	struct tda998x_priv *priv = dev_get_drvdata(dev);
 
@@ -1724,6 +1731,19 @@ static void tda998x_unbind(struct device *dev, struct device *master,
 	tda998x_destroy(priv);
 }
 
+static int tda998x_bind(struct device *dev, struct device *master, void *data)
+{
+	struct drm_device *drm = data;
+
+	return tda998x_init(dev, drm);
+}
+
+static void tda998x_unbind(struct device *dev, struct device *master,
+			   void *data)
+{
+	tda998x_fini(dev);
+}
+
 static const struct component_ops tda998x_ops = {
 	.bind = tda998x_bind,
 	.unbind = tda998x_unbind,
-- 
2.11.0

^ permalink raw reply related

* [PATCH v5 7/7] drm/i2c: tda998x: register as a drm bridge
From: Peter Rosin @ 2018-05-23  9:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180523093122.27859-1-peda@axentia.se>

This makes this driver work with all(?) drivers that are not
componentized and instead expect to connect to a panel/bridge. That
said, the only one tested is atmel-hlcdc.

This hooks the relevant work function previously called by the encoder
and the component also to the bridge, since the encoder goes away when
connecting to the bridge interface of the driver and the equivalent of
bind/unbind of the component is handled by bridge attach/detach.

The lifetime requirements of a bridge and a component are slightly
different, which is the reason for struct tda998x_bridge.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 170 ++++++++++++++++++++++++++++++++------
 1 file changed, 143 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index d401b3d0095c..d47e49c2991a 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -36,6 +36,14 @@ struct tda998x_audio_port {
 	u8 config;		/* AP value */
 };
 
+struct tda998x_priv;
+
+struct tda998x_bridge {
+	struct tda998x_priv *priv;
+	struct device *dev;
+	struct drm_bridge bridge;
+};
+
 struct tda998x_priv {
 	struct i2c_client *cec;
 	struct i2c_client *hdmi;
@@ -63,6 +71,8 @@ struct tda998x_priv {
 	wait_queue_head_t edid_delay_waitq;
 	bool edid_delay_active;
 
+	struct tda998x_bridge *bridge;
+	bool local_encoder;
 	struct drm_encoder encoder;
 	struct drm_connector connector;
 
@@ -75,6 +85,9 @@ struct tda998x_priv {
 #define enc_to_tda998x_priv(x) \
 	container_of(x, struct tda998x_priv, encoder)
 
+#define bridge_to_tda998x_bridge(x) \
+	container_of(x, struct tda998x_bridge, bridge)
+
 /* The TDA9988 series of devices use a paged register scheme.. to simplify
  * things we encode the page # in upper bits of the register #.  To read/
  * write a given register, we need to make sure CURPAGE register is set
@@ -842,7 +855,8 @@ static int tda998x_audio_hw_params(struct device *dev, void *data,
 				   struct hdmi_codec_daifmt *daifmt,
 				   struct hdmi_codec_params *params)
 {
-	struct tda998x_priv *priv = dev_get_drvdata(dev);
+	struct tda998x_bridge *bridge = dev_get_drvdata(dev);
+	struct tda998x_priv *priv = bridge->priv;
 	int i, ret;
 	struct tda998x_audio_params audio = {
 		.sample_width = params->sample_width,
@@ -899,7 +913,8 @@ static int tda998x_audio_hw_params(struct device *dev, void *data,
 
 static void tda998x_audio_shutdown(struct device *dev, void *data)
 {
-	struct tda998x_priv *priv = dev_get_drvdata(dev);
+	struct tda998x_bridge *bridge = dev_get_drvdata(dev);
+	struct tda998x_priv *priv = bridge->priv;
 
 	mutex_lock(&priv->audio_mutex);
 
@@ -912,7 +927,8 @@ static void tda998x_audio_shutdown(struct device *dev, void *data)
 
 int tda998x_audio_digital_mute(struct device *dev, void *data, bool enable)
 {
-	struct tda998x_priv *priv = dev_get_drvdata(dev);
+	struct tda998x_bridge *bridge = dev_get_drvdata(dev);
+	struct tda998x_priv *priv = bridge->priv;
 
 	mutex_lock(&priv->audio_mutex);
 
@@ -925,7 +941,8 @@ int tda998x_audio_digital_mute(struct device *dev, void *data, bool enable)
 static int tda998x_audio_get_eld(struct device *dev, void *data,
 				 uint8_t *buf, size_t len)
 {
-	struct tda998x_priv *priv = dev_get_drvdata(dev);
+	struct tda998x_bridge *bridge = dev_get_drvdata(dev);
+	struct tda998x_priv *priv = bridge->priv;
 
 	mutex_lock(&priv->audio_mutex);
 	memcpy(buf, priv->connector.eld,
@@ -1126,7 +1143,10 @@ tda998x_connector_best_encoder(struct drm_connector *connector)
 {
 	struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
 
-	return &priv->encoder;
+	if (priv->local_encoder)
+		return &priv->encoder;
+	else
+		return priv->bridge->bridge.encoder;
 }
 
 static
@@ -1140,6 +1160,7 @@ static int tda998x_connector_init(struct tda998x_priv *priv,
 				  struct drm_device *drm)
 {
 	struct drm_connector *connector = &priv->connector;
+	struct drm_encoder *encoder;
 	int ret;
 
 	connector->interlace_allowed = 1;
@@ -1156,7 +1177,8 @@ static int tda998x_connector_init(struct tda998x_priv *priv,
 	if (ret)
 		return ret;
 
-	drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
+	encoder = tda998x_connector_best_encoder(&priv->connector);
+	drm_mode_connector_attach_encoder(&priv->connector, encoder);
 
 	return 0;
 }
@@ -1671,8 +1693,10 @@ static void tda998x_set_config(struct tda998x_priv *priv,
 	priv->audio_params = p->audio_params;
 }
 
-static int tda998x_init(struct device *dev, struct drm_device *drm)
+static int tda998x_init(struct device *dev, struct drm_device *drm,
+			bool local_encoder)
 {
+	struct tda998x_bridge *bridge = dev_get_drvdata(dev);
 	struct tda998x_encoder_params *params = dev->platform_data;
 	struct i2c_client *client = to_i2c_client(dev);
 	struct tda998x_priv *priv;
@@ -1683,18 +1707,22 @@ static int tda998x_init(struct device *dev, struct drm_device *drm)
 	if (!priv)
 		return -ENOMEM;
 
-	dev_set_drvdata(dev, priv);
+	bridge->priv = priv;
+	priv->bridge = bridge;
+	priv->local_encoder = local_encoder;
 
-	if (dev->of_node)
-		crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
+	if (local_encoder) {
+		if (dev->of_node)
+			crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
 
-	/* If no CRTCs were found, fall back to our old behaviour */
-	if (crtcs == 0) {
-		dev_warn(dev, "Falling back to first CRTC\n");
-		crtcs = 1 << 0;
-	}
+		/* If no CRTCs were found, fall back to our old behaviour */
+		if (crtcs == 0) {
+			dev_warn(dev, "Falling back to first CRTC\n");
+			crtcs = 1 << 0;
+		}
 
-	priv->encoder.possible_crtcs = crtcs;
+		priv->encoder.possible_crtcs = crtcs;
+	}
 
 	ret = tda998x_create(client, priv);
 	if (ret)
@@ -1703,11 +1731,15 @@ static int tda998x_init(struct device *dev, struct drm_device *drm)
 	if (!dev->of_node && params)
 		tda998x_set_config(priv, params);
 
-	drm_encoder_helper_add(&priv->encoder, &tda998x_encoder_helper_funcs);
-	ret = drm_encoder_init(drm, &priv->encoder, &tda998x_encoder_funcs,
-			       DRM_MODE_ENCODER_TMDS, NULL);
-	if (ret)
-		goto err_encoder;
+	if (local_encoder) {
+		drm_encoder_helper_add(&priv->encoder,
+				       &tda998x_encoder_helper_funcs);
+		ret = drm_encoder_init(drm, &priv->encoder,
+				       &tda998x_encoder_funcs,
+				       DRM_MODE_ENCODER_TMDS, NULL);
+		if (ret)
+			goto err_encoder;
+	}
 
 	ret = tda998x_connector_init(priv, drm);
 	if (ret)
@@ -1716,7 +1748,8 @@ static int tda998x_init(struct device *dev, struct drm_device *drm)
 	return 0;
 
 err_connector:
-	drm_encoder_cleanup(&priv->encoder);
+	if (local_encoder)
+		drm_encoder_cleanup(&priv->encoder);
 err_encoder:
 	tda998x_destroy(priv);
 	return ret;
@@ -1724,10 +1757,12 @@ static int tda998x_init(struct device *dev, struct drm_device *drm)
 
 static void tda998x_fini(struct device *dev)
 {
-	struct tda998x_priv *priv = dev_get_drvdata(dev);
+	struct tda998x_bridge *bridge = dev_get_drvdata(dev);
+	struct tda998x_priv *priv = bridge->priv;
 
 	drm_connector_cleanup(&priv->connector);
-	drm_encoder_cleanup(&priv->encoder);
+	if (priv->local_encoder)
+		drm_encoder_cleanup(&priv->encoder);
 	tda998x_destroy(priv);
 }
 
@@ -1735,7 +1770,7 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
 {
 	struct drm_device *drm = data;
 
-	return tda998x_init(dev, drm);
+	return tda998x_init(dev, drm, true);
 }
 
 static void tda998x_unbind(struct device *dev, struct device *master,
@@ -1749,19 +1784,100 @@ static const struct component_ops tda998x_ops = {
 	.unbind = tda998x_unbind,
 };
 
+/* DRM bridge functions */
+
+static int tda998x_bridge_attach(struct drm_bridge *dbridge)
+{
+	struct tda998x_bridge *bridge = bridge_to_tda998x_bridge(dbridge);
+	struct device *dev = bridge->dev;
+	struct drm_device *drm = bridge->bridge.dev;
+
+	return tda998x_init(dev, drm, false);
+}
+
+static void tda998x_bridge_detach(struct drm_bridge *dbridge)
+{
+	struct tda998x_bridge *bridge = bridge_to_tda998x_bridge(dbridge);
+	struct device *dev = bridge->dev;
+
+	tda998x_fini(dev);
+}
+
+static void tda998x_bridge_enable(struct drm_bridge *dbridge)
+{
+	struct tda998x_bridge *bridge = bridge_to_tda998x_bridge(dbridge);
+	struct tda998x_priv *priv = bridge->priv;
+
+	tda998x_enable(priv);
+}
+
+static void tda998x_bridge_disable(struct drm_bridge *dbridge)
+{
+	struct tda998x_bridge *bridge = bridge_to_tda998x_bridge(dbridge);
+	struct tda998x_priv *priv = bridge->priv;
+
+	tda998x_disable(priv);
+}
+
+static void tda998x_bridge_mode_set(struct drm_bridge *dbridge,
+				    struct drm_display_mode *mode,
+				    struct drm_display_mode *adjusted_mode)
+{
+	struct tda998x_bridge *bridge = bridge_to_tda998x_bridge(dbridge);
+	struct tda998x_priv *priv = bridge->priv;
+
+	tda998x_mode_set(priv, mode, adjusted_mode);
+}
+
+static const struct drm_bridge_funcs tda998x_bridge_funcs = {
+	.attach = tda998x_bridge_attach,
+	.detach = tda998x_bridge_detach,
+	.enable = tda998x_bridge_enable,
+	.disable = tda998x_bridge_disable,
+	.mode_set = tda998x_bridge_mode_set,
+};
+
 static int
 tda998x_probe(struct i2c_client *client, const struct i2c_device_id *id)
 {
+	struct device *dev = &client->dev;
+	struct tda998x_bridge *bridge;
+	int ret;
+
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
 		dev_warn(&client->dev, "adapter does not support I2C\n");
 		return -EIO;
 	}
-	return component_add(&client->dev, &tda998x_ops);
+
+	bridge = devm_kzalloc(dev, sizeof(*bridge), GFP_KERNEL);
+	if (!bridge)
+		return -ENOMEM;
+
+	bridge->dev = dev;
+	dev_set_drvdata(dev, bridge);
+
+	bridge->bridge.funcs = &tda998x_bridge_funcs;
+#ifdef CONFIG_OF
+	bridge->bridge.of_node = dev->of_node;
+#endif
+	drm_bridge_add(&bridge->bridge);
+
+	ret = component_add(dev, &tda998x_ops);
+
+	if (ret)
+		drm_bridge_remove(&bridge->bridge);
+
+	return ret;
 }
 
 static int tda998x_remove(struct i2c_client *client)
 {
-	component_del(&client->dev, &tda998x_ops);
+	struct device *dev = &client->dev;
+	struct tda998x_bridge *bridge = dev_get_drvdata(dev);
+
+	drm_bridge_remove(&bridge->bridge);
+	component_del(dev, &tda998x_ops);
+
 	return 0;
 }
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH V3 8/8] dt-bindings: stm32: add compatible for syscon
From: Christophe ROULLIER @ 2018-05-23  9:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522172238.GA26145@rob-hp-laptop>

On 05/22/2018 07:22 PM, Rob Herring wrote:
> On Mon, May 21, 2018 at 10:07:26AM +0200, Christophe Roullier wrote:
>> This patch describes syscon DT bindings.
>>
>> Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
>> ---
>>   Documentation/devicetree/bindings/arm/stm32.txt | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/stm32.txt b/Documentation/devicetree/bindings/arm/stm32.txt
>> index 6808ed9..e46ebad 100644
>> --- a/Documentation/devicetree/bindings/arm/stm32.txt
>> +++ b/Documentation/devicetree/bindings/arm/stm32.txt
>> @@ -8,3 +8,8 @@ using one of the following compatible strings:
>>     st,stm32f746
>>     st,stm32h743
>>     st,stm32mp157
>> +
>> +Required nodes:
>> +- syscon: the soc bus node must have a system controller node pointing to the
>> +  global control registers, with the compatible string
>> +  "st,stm32mp157-syscfg", "syscon";
> 
> Please don't mix soc/board bindings with other nodes. So perhaps
> stm32-syscon.txt.
> 
> Rob
> 

Hi Rob,

Is it ok for you with this tree file:

Documentation/devicetree/bindings/arm/stm32/stm32.txt
Documentation/devicetree/bindings/arm/stm32/stm32-syscon.txt
	With stm32-syscon.txt:

---------------------------------------------------
STMicroelectronics STM32 Platforms System Controller

Properties:
  - compatible : should contain two values. First value must be :
		- " st,stm32mp157-syscfg " - for stm32mp157 based SoCs,
		second value must be always "syscon".
  - reg : offset and length of the register set.

Example:
	syscfg: system-config at 50020000 {
		compatible = "st,stm32mp157-syscfg", "syscon";
		reg = <0x50020000 0x400>;
	};
---------------------------------------------------

Do we need to update also all MCU family (stm32f4, stm32h7, stm32f7) 
property to be coherent ?

Thanks for your feedback.

Christophe.

^ permalink raw reply

* [PATCH] cpufreq: Add Kryo CPU scaling driver
From: Viresh Kumar @ 2018-05-23  9:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527066324-24726-1-git-send-email-ilialin@codeaurora.org>

On 23-05-18, 12:05, Ilia Lin wrote:
> In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors,
> the CPU frequency subset and voltage value of each OPP varies
> based on the silicon variant in use. Qualcomm Process Voltage Scaling Tables
> defines the voltage and frequency value based on the msm-id in SMEM
> and speedbin blown in the efuse combination.
> The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the SoC
> to provide the OPP framework with required information.
> This is used to determine the voltage and frequency value for each OPP of
> operating-points-v2 table when it is parsed by the OPP framework.
> 
> Signed-off-by: Ilia Lin <ilialin@codeaurora.org>
> ---
>  drivers/cpufreq/Kconfig.arm          |  10 +++
>  drivers/cpufreq/Makefile             |   1 +
>  drivers/cpufreq/cpufreq-dt-platdev.c |   3 +
>  drivers/cpufreq/qcom-cpufreq-kryo.c  | 163 +++++++++++++++++++++++++++++++++++
>  4 files changed, 177 insertions(+)
>  create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c

Hi Ilia,

So the patch looks good now. But I don't see how this series is going
to get merged. You are touching all parts of the kernel and no single
maintainer would be able to get these merged easily.

What I would suggest is that you divided the series in at least 3
parts.

- Patch 10 and 11, just the cpufreq stuff. So that Rafael can apply
  those independently. Also mention the dependency on my OPP patches
  in the cover letter for those two patches.

- All clk patches in another series, so that Stephen can apply them.

- And finally everything else to go via ARM-Soc.

Maybe things wouldn't be that simple, but that's what you would need
to do to get things merged. And please add all Acked by tags you got
to the patches.

For this patch:

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

--
viresh

^ permalink raw reply

* [PATCH 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Ulf Hansson @ 2018-05-23  9:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <00ba829d-faf4-168c-db00-531621b9280f@codeaurora.org>

On 23 May 2018 at 11:27, Rajendra Nayak <rnayak@codeaurora.org> wrote:
>
>
> On 05/23/2018 02:37 PM, Jon Hunter wrote:
>>
>> On 23/05/18 07:12, Ulf Hansson wrote:
>>
>> ...
>>
>>>>>>> Thanks for sending this. Believe it or not this has still been on my to-do list
>>>>>>> and so we definitely need a solution for Tegra.
>>>>>>>
>>>>>>> Looking at the above it appears that additional power-domains exposed as devices
>>>>>>> to the client device. So I assume that this means that the drivers for devices
>>>>>>> with multiple power-domains will need to call RPM APIs for each of these
>>>>>>> additional power-domains. Is that correct?
>>>>>>
>>>>>> They can, but should not!
>>>>>>
>>>>>> Instead, the driver shall use device_link_add() and device_link_del(),
>>>>>> dynamically, depending on what PM domain that their original device
>>>>>> needs for the current running use case.
>>>>>>
>>>>>> In that way, they keep existing runtime PM deployment, operating on
>>>>>> its original device.
>>>>>
>>>>> OK, sounds good. Any reason why the linking cannot be handled by the above API? Is there a use-case where you would not want it linked?
>>>>
>>>> I am guessing the linking is what would give the driver the ability to decide which subset of powerdomains it actually wants to control
>>>> at any point using runtime PM. If we have cases wherein the driver would want to turn on/off _all_ its associated powerdomains _always_
>>>> then a default linking of all would help.
>>>
>>> First, I think we need to decide on *where* the linking should be
>>> done, not at both places, as that would just mess up synchronization
>>> of who is responsible for calling the device_link_del() at detach.
>>>
>>> Second, It would in principle be fine to call device_link_add() and
>>> device_link_del() as a part of the attach/detach APIs. However, there
>>> is a downside to such solution, which would be that the driver then
>>> needs call the detach API, just to do device_link_del(). Of course
>>> then it would also needs to call the attach API later if/when needed.
>>> Doing this adds unnecessary overhead - comparing to just let the
>>> driver call device_link_add|del() when needed. On the upside, yes, it
>>> would put less burden on the drivers as it then only needs to care
>>> about using one set of functions.
>>>
>>> Which solution do you prefer?
>>
>> Any reason why we could not add a 'boolean' argument to the API to indicate whether the new device should be linked? I think that I prefer the API handles it, but I can see there could be instances where drivers may wish to handle it themselves.
>>
>> Rajendra, do you have a use-case right now where the driver would want to handle the linking?
>
> So if I understand this right, any driver which does want to control individual powerdomain state would
> need to do the linking itself right?
>
> What I am saying is, if I have device A, with powerdomains X and Y, and if I want to turn on only X,
> then I would want only X to be linked with A, and at a later point if I want both X and Y to be turned on,
> I would then go ahead and link both X and Y to A? Is that correct or did I get it all wrong?

Correct!

>
> I know atleast Camera on msm8996 would need to do this since it has 2 vfe powerdoamins, which can be
> turned on one at a time (depending on what resolution needs to be supported) or both together if we
> really need very high resolution using both vfe modules.

I think this is also the case for the Tegra XUSB subsystem.

The usb device is always attached to one PM domain, but depending on
if super-speed mode is used, another PM domain for that logic needs to
be powered on as well.

Jon, please correct me if I am wrong!

Kind regards
Uffe

^ permalink raw reply

* [PATCH v2 13/40] vfio: Add support for Shared Virtual Addressing
From: Xu Zaibo @ 2018-05-23  9:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180511190641.23008-14-jean-philippe.brucker@arm.com>

Hi,

On 2018/5/12 3:06, Jean-Philippe Brucker wrote:
> Add two new ioctls for VFIO containers. VFIO_IOMMU_BIND_PROCESS creates a
> bond between a container and a process address space, identified by a
> Process Address Space ID (PASID). Devices in the container append this
> PASID to DMA transactions in order to access the process' address space.
> The process page tables are shared with the IOMMU, and mechanisms such as
> PCI ATS/PRI are used to handle faults. VFIO_IOMMU_UNBIND_PROCESS removes a
> bond created with VFIO_IOMMU_BIND_PROCESS.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
>
>
>
>
>
>
> +static int vfio_iommu_bind_group(struct vfio_iommu *iommu,
> +				 struct vfio_group *group,
> +				 struct vfio_mm *vfio_mm)
> +{
> +	int ret;
> +	bool enabled_sva = false;
> +	struct vfio_iommu_sva_bind_data data = {
> +		.vfio_mm	= vfio_mm,
> +		.iommu		= iommu,
> +		.count		= 0,
> +	};
> +
> +	if (!group->sva_enabled) {
> +		ret = iommu_group_for_each_dev(group->iommu_group, NULL,
> +					       vfio_iommu_sva_init);
Do we need to do *sva_init here or do anything to avoid repeated initiation?
while another process already did initiation at this device, I think 
that current process will get an EEXIST.

Thanks.
> +		if (ret)
> +			return ret;
> +
> +		group->sva_enabled = enabled_sva = true;
> +	}
> +
> +	ret = iommu_group_for_each_dev(group->iommu_group, &data,
> +				       vfio_iommu_sva_bind_dev);
> +	if (ret && data.count > 1)
> +		iommu_group_for_each_dev(group->iommu_group, vfio_mm,
> +					 vfio_iommu_sva_unbind_dev);
> +	if (ret && enabled_sva) {
> +		iommu_group_for_each_dev(group->iommu_group, NULL,
> +					 vfio_iommu_sva_shutdown);
> +		group->sva_enabled = false;
> +	}
> +
> +	return ret;
> +}
>
>
>   
> @@ -1442,6 +1636,10 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
>   	if (ret)
>   		goto out_detach;
>   
> +	ret = vfio_iommu_replay_bind(iommu, group);
> +	if (ret)
> +		goto out_detach;
> +
>   	if (resv_msi) {
>   		ret = iommu_get_msi_cookie(domain->domain, resv_msi_base);
>   		if (ret)
> @@ -1547,6 +1745,11 @@ static void vfio_iommu_type1_detach_group(void *iommu_data,
>   			continue;
>   
>   		iommu_detach_group(domain->domain, iommu_group);
> +		if (group->sva_enabled) {
> +			iommu_group_for_each_dev(iommu_group, NULL,
> +						 vfio_iommu_sva_shutdown);
> +			group->sva_enabled = false;
> +		}
Here, why shut down here? If another process is working on the device, 
there may be a crash?

Thanks.
>   		list_del(&group->next);
>   		kfree(group);
>   		/*
> @@ -1562,6 +1765,7 @@ static void vfio_iommu_type1_detach_group(void *iommu_data,
>

^ permalink raw reply

* [PATCH] cpufreq: Add Kryo CPU scaling driver
From: Russell King - ARM Linux @ 2018-05-23  9:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527066324-24726-1-git-send-email-ilialin@codeaurora.org>

On Wed, May 23, 2018 at 12:05:24PM +0300, Ilia Lin wrote:
> +	np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
> +	if (IS_ERR(np))
> +		return PTR_ERR(np);
...
> +
> +	pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
> +	if (!IS_ERR(pdev))

Do you need to hold a reference to `np' here?

> +		return 0;

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

^ permalink raw reply

* [PATCH] gpio: zynq: Setup chip->base based on alias ID
From: Linus Walleij @ 2018-05-23  9:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <57e4bbdd-1e30-0352-f758-998b64a6b77f@xilinx.com>

On Wed, May 2, 2018 at 4:19 PM, Michal Simek <michal.simek@xilinx.com> wrote:
> On 2.5.2018 15:56, Linus Walleij wrote:

>> Would it be possible that I apply the patch, and somehow also
>> establish some understanding with all users of the Xilinx
>> platform that whatever legacy applications are out there
>> must start to migrate towards using the character device so
>> this reliance on the numberspace doesn't stick around forever?
>
> When someone contacts me for asking guidance for gpio I am telling them
> not to use legacy sysfs interface and use libgpiod.

Awesome, thanks for your support :)

> Last one was a week
> ago in connection to Ultra96 and libmraa.
>
> But even chardev is not supported there now.
> https://github.com/intel-iot-devkit/mraa/issues/713

Hm I hope this happens soon. Manavanninan is doing a great
job in switching this over, I just have to accept that the pace
isn't always what it should be in all upstream projects, and
deal with an imperfect world.

> If you take a look at
> arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
> which is Ultra96 board gpio-line-names are filled there for the whole PS
> part. Definitely take a look and let know if you find out any issue there.

It looks good. I even managed to boot my Ultra96 board which
was sent over as price in some competition (great hardware!)

> zynq/zynqmp gpio controller contains PS pins (hard part) and PL pins
> coming to logic.
>
> I can't describe PL gpio pins because it can be whatever even I have
> done that for one fixed hw design.
>
> Interesting part on that sha1 you shared is how "NC" pin is described.
>
> gpio pin 35 I have on zcu100 as "" but it should be maybe TP_PAD which
> is really just a pad on real board. And the rest of "" gpio names are
> connected to PL.

How to handle anything routed to/from programmable logic is
in a bit of mess right now, I understand this work isn't the
easiest :/

Let's go for this patch and try to improve the situation gradually
moving forward.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH] gpio: zynq: Setup chip->base based on alias ID
From: Linus Walleij @ 2018-05-23  9:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <8b344d48-592d-8be8-bea0-e29b3e8469a1@xilinx.com>

On Tue, May 15, 2018 at 3:26 PM, Michal Simek <michal.simek@xilinx.com> wrote:

> Have you done any decision about this patch?

Yeah I applied the patch. It's a bit of "choose the lesser evil" approach
but what can I do...

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Jon Hunter @ 2018-05-23  9:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAPDyKFoqLPRwEJ9b+XBpB4Zy79SGEC-a8V8shNhMkYOCQa0=oA@mail.gmail.com>


On 23/05/18 10:33, Ulf Hansson wrote:
> On 23 May 2018 at 11:27, Rajendra Nayak <rnayak@codeaurora.org> wrote:
>>
>>
>> On 05/23/2018 02:37 PM, Jon Hunter wrote:
>>>
>>> On 23/05/18 07:12, Ulf Hansson wrote:
>>>
>>> ...
>>>
>>>>>>>> Thanks for sending this. Believe it or not this has still been on my to-do list
>>>>>>>> and so we definitely need a solution for Tegra.
>>>>>>>>
>>>>>>>> Looking at the above it appears that additional power-domains exposed as devices
>>>>>>>> to the client device. So I assume that this means that the drivers for devices
>>>>>>>> with multiple power-domains will need to call RPM APIs for each of these
>>>>>>>> additional power-domains. Is that correct?
>>>>>>>
>>>>>>> They can, but should not!
>>>>>>>
>>>>>>> Instead, the driver shall use device_link_add() and device_link_del(),
>>>>>>> dynamically, depending on what PM domain that their original device
>>>>>>> needs for the current running use case.
>>>>>>>
>>>>>>> In that way, they keep existing runtime PM deployment, operating on
>>>>>>> its original device.
>>>>>>
>>>>>> OK, sounds good. Any reason why the linking cannot be handled by the above API? Is there a use-case where you would not want it linked?
>>>>>
>>>>> I am guessing the linking is what would give the driver the ability to decide which subset of powerdomains it actually wants to control
>>>>> at any point using runtime PM. If we have cases wherein the driver would want to turn on/off _all_ its associated powerdomains _always_
>>>>> then a default linking of all would help.
>>>>
>>>> First, I think we need to decide on *where* the linking should be
>>>> done, not at both places, as that would just mess up synchronization
>>>> of who is responsible for calling the device_link_del() at detach.
>>>>
>>>> Second, It would in principle be fine to call device_link_add() and
>>>> device_link_del() as a part of the attach/detach APIs. However, there
>>>> is a downside to such solution, which would be that the driver then
>>>> needs call the detach API, just to do device_link_del(). Of course
>>>> then it would also needs to call the attach API later if/when needed.
>>>> Doing this adds unnecessary overhead - comparing to just let the
>>>> driver call device_link_add|del() when needed. On the upside, yes, it
>>>> would put less burden on the drivers as it then only needs to care
>>>> about using one set of functions.
>>>>
>>>> Which solution do you prefer?
>>>
>>> Any reason why we could not add a 'boolean' argument to the API to indicate whether the new device should be linked? I think that I prefer the API handles it, but I can see there could be instances where drivers may wish to handle it themselves.
>>>
>>> Rajendra, do you have a use-case right now where the driver would want to handle the linking?
>>
>> So if I understand this right, any driver which does want to control individual powerdomain state would
>> need to do the linking itself right?
>>
>> What I am saying is, if I have device A, with powerdomains X and Y, and if I want to turn on only X,
>> then I would want only X to be linked with A, and at a later point if I want both X and Y to be turned on,
>> I would then go ahead and link both X and Y to A? Is that correct or did I get it all wrong?
> 
> Correct!
> 
>>
>> I know atleast Camera on msm8996 would need to do this since it has 2 vfe powerdoamins, which can be
>> turned on one at a time (depending on what resolution needs to be supported) or both together if we
>> really need very high resolution using both vfe modules.
> 
> I think this is also the case for the Tegra XUSB subsystem.
> 
> The usb device is always attached to one PM domain, but depending on
> if super-speed mode is used, another PM domain for that logic needs to
> be powered on as well.
> 
> Jon, please correct me if I am wrong!

Yes this is technically correct, however, in reality I think we are 
always going to enable the superspeed domain if either the host or 
device domain is enabled. So we would probably always link the 
superspeed with the host and device devices.

Cheers
Jon

-- 
nvpublic

^ permalink raw reply

* [PATCH 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Ulf Hansson @ 2018-05-23  9:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <22068fb1-f6e8-f2ec-d7f4-ab9e93469d7f@nvidia.com>

On 23 May 2018 at 11:45, Jon Hunter <jonathanh@nvidia.com> wrote:
>
> On 23/05/18 10:33, Ulf Hansson wrote:
>>
>> On 23 May 2018 at 11:27, Rajendra Nayak <rnayak@codeaurora.org> wrote:
>>>
>>>
>>>
>>> On 05/23/2018 02:37 PM, Jon Hunter wrote:
>>>>
>>>>
>>>> On 23/05/18 07:12, Ulf Hansson wrote:
>>>>
>>>> ...
>>>>
>>>>>>>>> Thanks for sending this. Believe it or not this has still been on
>>>>>>>>> my to-do list
>>>>>>>>> and so we definitely need a solution for Tegra.
>>>>>>>>>
>>>>>>>>> Looking at the above it appears that additional power-domains
>>>>>>>>> exposed as devices
>>>>>>>>> to the client device. So I assume that this means that the drivers
>>>>>>>>> for devices
>>>>>>>>> with multiple power-domains will need to call RPM APIs for each of
>>>>>>>>> these
>>>>>>>>> additional power-domains. Is that correct?
>>>>>>>>
>>>>>>>>
>>>>>>>> They can, but should not!
>>>>>>>>
>>>>>>>> Instead, the driver shall use device_link_add() and
>>>>>>>> device_link_del(),
>>>>>>>> dynamically, depending on what PM domain that their original device
>>>>>>>> needs for the current running use case.
>>>>>>>>
>>>>>>>> In that way, they keep existing runtime PM deployment, operating on
>>>>>>>> its original device.
>>>>>>>
>>>>>>>
>>>>>>> OK, sounds good. Any reason why the linking cannot be handled by the
>>>>>>> above API? Is there a use-case where you would not want it linked?
>>>>>>
>>>>>>
>>>>>> I am guessing the linking is what would give the driver the ability to
>>>>>> decide which subset of powerdomains it actually wants to control
>>>>>> at any point using runtime PM. If we have cases wherein the driver
>>>>>> would want to turn on/off _all_ its associated powerdomains _always_
>>>>>> then a default linking of all would help.
>>>>>
>>>>>
>>>>> First, I think we need to decide on *where* the linking should be
>>>>> done, not at both places, as that would just mess up synchronization
>>>>> of who is responsible for calling the device_link_del() at detach.
>>>>>
>>>>> Second, It would in principle be fine to call device_link_add() and
>>>>> device_link_del() as a part of the attach/detach APIs. However, there
>>>>> is a downside to such solution, which would be that the driver then
>>>>> needs call the detach API, just to do device_link_del(). Of course
>>>>> then it would also needs to call the attach API later if/when needed.
>>>>> Doing this adds unnecessary overhead - comparing to just let the
>>>>> driver call device_link_add|del() when needed. On the upside, yes, it
>>>>> would put less burden on the drivers as it then only needs to care
>>>>> about using one set of functions.
>>>>>
>>>>> Which solution do you prefer?
>>>>
>>>>
>>>> Any reason why we could not add a 'boolean' argument to the API to
>>>> indicate whether the new device should be linked? I think that I prefer the
>>>> API handles it, but I can see there could be instances where drivers may
>>>> wish to handle it themselves.
>>>>
>>>> Rajendra, do you have a use-case right now where the driver would want
>>>> to handle the linking?
>>>
>>>
>>> So if I understand this right, any driver which does want to control
>>> individual powerdomain state would
>>> need to do the linking itself right?
>>>
>>> What I am saying is, if I have device A, with powerdomains X and Y, and
>>> if I want to turn on only X,
>>> then I would want only X to be linked with A, and at a later point if I
>>> want both X and Y to be turned on,
>>> I would then go ahead and link both X and Y to A? Is that correct or did
>>> I get it all wrong?
>>
>>
>> Correct!
>>
>>>
>>> I know atleast Camera on msm8996 would need to do this since it has 2 vfe
>>> powerdoamins, which can be
>>> turned on one at a time (depending on what resolution needs to be
>>> supported) or both together if we
>>> really need very high resolution using both vfe modules.
>>
>>
>> I think this is also the case for the Tegra XUSB subsystem.
>>
>> The usb device is always attached to one PM domain, but depending on
>> if super-speed mode is used, another PM domain for that logic needs to
>> be powered on as well.
>>
>> Jon, please correct me if I am wrong!
>
>
> Yes this is technically correct, however, in reality I think we are always
> going to enable the superspeed domain if either the host or device domain is
> enabled. So we would probably always link the superspeed with the host and
> device devices.

Why? Wouldn't that waste power if the superspeed mode isn't used?

Kind regards
Uffe

^ permalink raw reply

* [PATCH] cpufreq: Add Kryo CPU scaling driver
From: Viresh Kumar @ 2018-05-23  9:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180523094033.GW17671@n2100.armlinux.org.uk>

On 23-05-18, 10:40, Russell King - ARM Linux wrote:
> On Wed, May 23, 2018 at 12:05:24PM +0300, Ilia Lin wrote:
> > +	np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
> > +	if (IS_ERR(np))
> > +		return PTR_ERR(np);
> ...
> > +
> > +	pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
> > +	if (!IS_ERR(pdev))
> 
> Do you need to hold a reference to `np' here?

I am starting to feel bad for Ilia now. The problem is that there was
a lot of stuff wrong with the patch and even with so many reviewers it
wasn't easy to notice all the problems it had.

But you are right, this reference needs to be dropped.

-- 
viresh

^ permalink raw reply

* [PATCH v3 00/13] add power domain support for Rockchip Socs
From: Ulf Hansson @ 2018-05-23 10:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527058129-10260-1-git-send-email-zhangqing@rock-chips.com>

On 23 May 2018 at 08:48, Elaine Zhang <zhangqing@rock-chips.com> wrote:
> add power domain support for RK3036/RK3128/RK3228/PX30 Soc.
> fix up the wrong value when set power domain up.
>
> Change in V2:
> Fix up the commit message description and Assign author.
>
> Change in V3:
> [PATCH 01/13]: The Copyright description use SPDX tag instead.
> [PATCH 05/13]: The Copyright description use SPDX tag instead.
> [PATCH 08/13]: The Copyright description use SPDX tag instead.
> [PATCH 11/13]: The Copyright description use SPDX tag instead.
>
> Caesar Wang (3):
>   dt-bindings: power: add RK3036 SoCs header for power-domain
>   dt-bindings: add binding for rk3036 power domains
>   Soc: rockchip: power-domain: add power domain support for rk3036
>
> Elaine Zhang (6):
>   dt-bindings: power: add RK3128 SoCs header for power-domain
>   dt-bindings: add binding for rk3128 power domains
>   soc: rockchip: power-domain: add power domain support for rk3128
>   dt-bindings: power: add RK3228 SoCs header for power-domain
>   dt-bindings: add binding for rk3228 power domains
>   soc: rockchip: power-domain: add power domain support for rk3228
>
> Finley Xiao (4):
>   soc: rockchip: power-domain: Fix wrong value when power up pd
>   dt-bindings: power: add PX30 SoCs header for power-domain
>   dt-bindings: add binding for px30 power domains
>   soc: rockchip: power-domain: add power domain support for px30
>
>  .../bindings/soc/rockchip/power_domain.txt         |  12 +++
>  drivers/soc/rockchip/pm_domains.c                  | 116 ++++++++++++++++++++-
>  include/dt-bindings/power/px30-power.h             |  27 +++++
>  include/dt-bindings/power/rk3036-power.h           |  13 +++
>  include/dt-bindings/power/rk3128-power.h           |  14 +++
>  include/dt-bindings/power/rk3228-power.h           |  21 ++++
>  6 files changed, 202 insertions(+), 1 deletion(-)
>  create mode 100644 include/dt-bindings/power/px30-power.h
>  create mode 100644 include/dt-bindings/power/rk3036-power.h
>  create mode 100644 include/dt-bindings/power/rk3128-power.h
>  create mode 100644 include/dt-bindings/power/rk3228-power.h
>
> --
> 1.9.1
>
>

Seems like the changes in v3 is very small, so feel free to add, for the series:

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

^ permalink raw reply

* [PATCH 03/14] arm64: Add per-cpu infrastructure to call ARCH_WORKAROUND_2
From: Julien Grall @ 2018-05-23 10:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522150648.28297-4-marc.zyngier@arm.com>

Hi Marc,

On 05/22/2018 04:06 PM, Marc Zyngier wrote:
> In a heterogeneous system, we can end up with both affected and
> unaffected CPUs. Let's check their status before calling into the
> firmware.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

Reviewed-by: Julien Grall <julien.grall@arm.com>

Cheers,

> ---
>   arch/arm64/kernel/cpu_errata.c |  2 ++
>   arch/arm64/kernel/entry.S      | 11 +++++++----
>   2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 46b3aafb631a..0288d6cf560e 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -233,6 +233,8 @@ enable_smccc_arch_workaround_1(const struct arm64_cpu_capabilities *entry)
>   #endif	/* CONFIG_HARDEN_BRANCH_PREDICTOR */
>   
>   #ifdef CONFIG_ARM64_SSBD
> +DEFINE_PER_CPU_READ_MOSTLY(u64, arm64_ssbd_callback_required);
> +
>   void __init arm64_update_smccc_conduit(struct alt_instr *alt,
>   				       __le32 *origptr, __le32 *updptr,
>   				       int nr_inst)
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index f33e6aed3037..29ad672a6abd 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -140,8 +140,10 @@ alternative_else_nop_endif
>   
>   	// This macro corrupts x0-x3. It is the caller's duty
>   	// to save/restore them if required.
> -	.macro	apply_ssbd, state
> +	.macro	apply_ssbd, state, targ, tmp1, tmp2
>   #ifdef CONFIG_ARM64_SSBD
> +	ldr_this_cpu	\tmp2, arm64_ssbd_callback_required, \tmp1
> +	cbz	\tmp2, \targ
>   	mov	w0, #ARM_SMCCC_ARCH_WORKAROUND_2
>   	mov	w1, #\state
>   alternative_cb	arm64_update_smccc_conduit
> @@ -176,12 +178,13 @@ alternative_cb_end
>   	ldr	x19, [tsk, #TSK_TI_FLAGS]	// since we can unmask debug
>   	disable_step_tsk x19, x20		// exceptions when scheduling.
>   
> -	apply_ssbd 1
> +	apply_ssbd 1, 1f, x22, x23
>   
>   #ifdef CONFIG_ARM64_SSBD
>   	ldp	x0, x1, [sp, #16 * 0]
>   	ldp	x2, x3, [sp, #16 * 1]
>   #endif
> +1:
>   
>   	mov	x29, xzr			// fp pointed to user-space
>   	.else
> @@ -323,8 +326,8 @@ alternative_if ARM64_WORKAROUND_845719
>   alternative_else_nop_endif
>   #endif
>   3:
> -	apply_ssbd 0
> -
> +	apply_ssbd 0, 5f, x0, x1
> +5:
>   	.endif
>   
>   	msr	elr_el1, x21			// set up the return data
> 

-- 
Julien Grall

^ permalink raw reply


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