* [PATCH v4] drm/mediatek: fixed the calc method of data rate per lane
From: Jitao Shi @ 2016-10-27 2:21 UTC (permalink / raw)
To: Philipp Zabel, ck.hu, Matthias Brugger
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Jitao Shi, Ajay Kumar, Inki Dae, Rahul Sharma, Sean Paul,
Vincent Palatin, Andy Yan, Russell King, devicetree, linux-kernel,
dri-devel, linux-arm-kernel, linux-mediatek, srv_heupstream,
Sascha Hauer, yingjoe.chen
Tune dsi frame rate by pixel clock, dsi add some extra signal (i.e.
Tlpx, Ths-prepare, Ths-zero, Ths-trail,Ths-exit) when enter and exit LP
mode, those signals will cause h-time larger than normal and reduce FPS.
So need to multiply a coefficient to offset the extra signal's effect.
coefficient = ((htotal*bpp/lane_number)+Tlpx+Ths_prep+Ths_zero+
Ths_trail+Ths_exit)/(htotal*bpp/lane_number)
Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
---
Chnage since v3:
- wrapp the commit msg.
- fix alignment of some lines.
Change since v2:
- move phy timing back to dsi_phy_timconfig.
Change since v1:
- phy_timing2 and phy_timing3 refer clock cycle time.
- define values of LPX HS_PRPR HS_ZERO HS_TRAIL TA_GO TA_SURE TA_GET DA_HS_EXIT.
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 71 +++++++++++++++++++++++++-----------
1 file changed, 49 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 28b2044..5defe58 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -85,16 +85,16 @@
#define LD0_WAKEUP_EN BIT(2)
#define DSI_PHY_TIMECON0 0x110
-#define LPX (0xff << 0)
-#define HS_PRPR (0xff << 8)
-#define HS_ZERO (0xff << 16)
-#define HS_TRAIL (0xff << 24)
+#define LPX (5 << 0)
+#define HS_PRPR (6 << 8)
+#define HS_ZERO (10 << 16)
+#define HS_TRAIL (8 << 24)
#define DSI_PHY_TIMECON1 0x114
-#define TA_GO (0xff << 0)
-#define TA_SURE (0xff << 8)
-#define TA_GET (0xff << 16)
-#define DA_HS_EXIT (0xff << 24)
+#define TA_GO (20 << 0)
+#define TA_SURE (7 << 8)
+#define TA_GET (25 << 16)
+#define DA_HS_EXIT (7 << 24)
#define DSI_PHY_TIMECON2 0x118
#define CONT_DET (0xff << 0)
@@ -161,20 +161,17 @@ static void mtk_dsi_mask(struct mtk_dsi *dsi, u32 offset, u32 mask, u32 data)
static void dsi_phy_timconfig(struct mtk_dsi *dsi)
{
u32 timcon0, timcon1, timcon2, timcon3;
- unsigned int ui, cycle_time;
- unsigned int lpx;
+ u32 ui, cycle_time;
ui = 1000 / dsi->data_rate + 0x01;
cycle_time = 8000 / dsi->data_rate + 0x01;
- lpx = 5;
- timcon0 = (8 << 24) | (0xa << 16) | (0x6 << 8) | lpx;
- timcon1 = (7 << 24) | (5 * lpx << 16) | ((3 * lpx) / 2) << 8 |
- (4 * lpx);
+ timcon0 = LPX | HS_PRPR | HS_ZERO | HS_TRAIL;
+ timcon1 = 4 * LPX | (3 * LPX / 2) << 8 | 5 * LPX << 16 | DA_HS_EXIT;
timcon2 = ((NS_TO_CYCLE(0x64, cycle_time) + 0xa) << 24) |
(NS_TO_CYCLE(0x150, cycle_time) << 16);
- timcon3 = (2 * lpx) << 16 | NS_TO_CYCLE(80 + 52 * ui, cycle_time) << 8 |
- NS_TO_CYCLE(0x40, cycle_time);
+ timcon3 = NS_TO_CYCLE(0x40, cycle_time) | (2 * LPX) << 16 |
+ NS_TO_CYCLE(80 + 52 * ui, cycle_time) << 8;
writel(timcon0, dsi->regs + DSI_PHY_TIMECON0);
writel(timcon1, dsi->regs + DSI_PHY_TIMECON1);
@@ -202,19 +199,49 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
{
struct device *dev = dsi->dev;
int ret;
+ u64 bit_clock, total_bits;
+ u32 htotal, htotal_bits, bit_per_pixel, overhead_cycles, overhead_bits;
if (++dsi->refcount != 1)
return 0;
+ switch (dsi->format) {
+ case MIPI_DSI_FMT_RGB565:
+ bit_per_pixel = 16;
+ break;
+ case MIPI_DSI_FMT_RGB666_PACKED:
+ bit_per_pixel = 18;
+ break;
+ case MIPI_DSI_FMT_RGB666:
+ case MIPI_DSI_FMT_RGB888:
+ default:
+ bit_per_pixel = 24;
+ break;
+ }
/**
- * data_rate = (pixel_clock / 1000) * pixel_dipth * mipi_ratio;
- * pixel_clock unit is Khz, data_rata unit is MHz, so need divide 1000.
- * mipi_ratio is mipi clk coefficient for balance the pixel clk in mipi.
- * we set mipi_ratio is 1.05.
+ * data_rate = (pixel_clock) * bit_per_pixel * mipi_ratio / lane_num;
+ * vm.pixelclock is Khz, data_rata unit is Hz, so need to multiply 1000
+ * mipi_ratio is (htotal * byte_per_pixel / lane_num + Tlpx + Ths_prep
+ * + Thstrail + Ths_exit + Ths_zero) /
+ * (htotal * byte_per_pixel /lane_number)
*/
- dsi->data_rate = dsi->vm.pixelclock * 3 * 21 / (1 * 1000 * 10);
+ bit_clock = dsi->vm.pixelclock * 1000 * bit_per_pixel;
+ htotal = dsi->vm.hactive + dsi->vm.hback_porch + dsi->vm.hfront_porch +
+ dsi->vm.hsync_len;
+ htotal_bits = htotal * bit_per_pixel;
+
+ /**
+ * overhead = lpx + hs_prepare + hs_zero + hs_trail + hs_exit
+ */
+ overhead_cycles = LPX + (HS_PRPR >> 8) + (HS_ZERO >> 16) +
+ (HS_TRAIL >> 24) + (DA_HS_EXIT >> 24);
+ overhead_bits = overhead_cycles * dsi->lanes * 8;
+ total_bits = htotal_bits + overhead_bits;
+
+ dsi->data_rate = DIV_ROUND_UP_ULL(bit_clock * total_bits,
+ htotal_bits * dsi->lanes);
- ret = clk_set_rate(dsi->hs_clk, dsi->data_rate * 1000000);
+ ret = clk_set_rate(dsi->hs_clk, dsi->data_rate);
if (ret < 0) {
dev_err(dev, "Failed to set data rate: %d\n", ret);
goto err_refcount;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v7 2/4] soc: mediatek: Init MT8173 scpsys driver earlier
From: Matthias Brugger @ 2016-10-26 14:54 UTC (permalink / raw)
To: Yong Wu
Cc: James Liao, Rob Herring, srv_heupstream-NuS5LvNUpcJWk0Htik3J/w,
devicetree-u79uwXL29TY76Z2rM5mHXA, Kevin Hilman,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sascha Hauer,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
joro-zLv9SwRftAIdnm+yROfE0A,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
In-Reply-To: <1468314071.24705.4.camel@mhfsdcap03>
Hi Yong,
On 07/12/2016 11:01 AM, Yong Wu wrote:
> Hi Matthias,
>
> On Fri, 2016-07-08 at 14:47 +0200, Matthias Brugger wrote:
>>
>> On 06/07/16 07:22, James Liao wrote:
>>> On Sat, 2016-07-02 at 18:35 +0200, Matthias Brugger wrote:
>>>>
>>>> On 05/16/2016 11:28 AM, James Liao wrote:
>>>>> Some power domain comsumers may init before module_init.
>>>>> So the power domain provider (scpsys) need to be initialized
>>>>> earlier too.
>>>>>
>>>>> Take an example for our IOMMU (M4U) and SMI. SMI is a bridge
>>>>> between IOMMU and multimedia HW. SMI is responsible to
>>>>> enable/disable iommu and help transfer data for each multimedia
>>>>> HW. Both of them have to wait until the power and clocks are
>>>>> enabled.
>>>>>
>>>>> So scpsys driver should be initialized before SMI, and SMI should
>>>>> be initialized before IOMMU, and then init IOMMU consumers
>>>>> (display/vdec/venc/camera etc.).
>>>>>
>>>>> IOMMU is subsys_init by default. So we need to init scpsys driver
>>>>> before subsys_init.
>>>>>
>>>>> Signed-off-by: James Liao <jamesjj.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>>>>> Reviewed-by: Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>>>>> ---
>>>>> drivers/soc/mediatek/mtk-scpsys.c | 19 ++++++++++++++++++-
>>>>> 1 file changed, 18 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
>>>>> index 5870a24..00c0adb 100644
>>>>> --- a/drivers/soc/mediatek/mtk-scpsys.c
>>>>> +++ b/drivers/soc/mediatek/mtk-scpsys.c
>>>>> @@ -617,4 +617,21 @@ static struct platform_driver scpsys_drv = {
>>>>> .of_match_table = of_match_ptr(of_scpsys_match_tbl),
>>>>> },
>>>>> };
>>>>> -builtin_platform_driver(scpsys_drv);
>>>>> +
>>>>> +static int __init scpsys_drv_init(void)
>>>>> +{
>>>>> + return platform_driver_register(&scpsys_drv);
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * There are some Mediatek drivers which depend on the power domain driver need
>>>>> + * to probe in earlier initcall levels. So scpsys driver also need to probe
>>>>> + * earlier.
>>>>> + *
>>>>> + * IOMMU(M4U) and SMI drivers for example. SMI is a bridge between IOMMU and
>>>>> + * multimedia HW. IOMMU depends on SMI, and SMI is a power domain consumer,
>>>>> + * so the proper probe sequence should be scpsys -> SMI -> IOMMU driver.
>>>>> + * IOMMU drivers are initialized during subsys_init by default, so we need to
>>>>> + * move SMI and scpsys drivers to subsys_init or earlier init levels.
>>>>> + */
>>>>> +subsys_initcall(scpsys_drv_init);
>>>>>
>>>>
>>>> Can't we achieve this with probe deferring? I'm not really keen on
>>>> coding the order of the different drivers like this.
>>>
>>> Hi Matthias,
>>>
>>> Some drivers such as IOMMU don't support probe deferring. So scpsys need
>>> to init before them by changing init level.
>>>
>>
>> SMI larbs return EPROBE_DEFER if it can't find the PM provider for it's
>> domain, so this part should not be the problem. The larbs get added as
>> components in the probe, when the PM provider is present.
>>
>> The iommu driver uses the larbs as components. As long as not all
>> components are present the iommu does not bind them. So from what I see
>> this should work without any problem.
>>
>> Can you please specify where the iommu dirver has a problem. Maybe we
>> need to fix the driver rather then scpsys.
>
> We got a problem while bootup, the hang backtrace is like below(Sorry, I
> can't find the full backtrace now):
>
> [ 7.832359] Call trace:
> [ 7.834778] [<ffffffc000764424>] mtk_smi_larb_get+0x24/0xa8
> [ 7.840300] [<ffffffc0005a1390>] mtk_drm_crtc_enable+0x6c/0x450
>
> The reason is that "larb->mmu" is NULL while DRM call mtk_smi_larb_get.
>
> DRM call iommu_present to wait for whether IOMMU is ready[1].
> But in the MTK-IOMMU, It will call
> bus_set_iommu->mtk_iommu_add_device->mtk_iommu_attach_device, then it's
> able to transfer "struct mtk_smi_iommu" to SMI whose probe maybe delayed
> by power-domain, then SMI could finish its probe.
>
> So If DRM probe is called after the time of calling bus_set_iommu and
> before the time of SMI probe finish, this problem can be reproduced.
>
> The root cause is that iommu_present cann't indicate that MTK-IOMMU and
> SMI is ready actually. in other words, the DRM cann't detect
> whether the SMI has probed done.
>
> If we move scpsys_init from module_initcall to subsys_initcall, the
> IOMMU and SMI could be probe done during the subsys_initcall period.
> this issue can be avoid.
> And as we grep before, there are also some examples whose power-domain
> is also not module_init[2].
> core_initcall(exynos4_pm_init_power_domain);
> subsys_initcall(imx_pgc_init);
>
> If you think that this patch will hide the problem above, then I have to
> add a new function, like below:
> //==================
> bool mtk_smi_larb_ready(struct device *larbdev)
> {
> struct mtk_smi_larb *larb = dev_get_drvdata(larbdev);
>
> return larb && !!larb->mmu;
> }
> //=================
>
> And in the probe of all the MTK-IOMMU consumer, we have to add this:
> //====================
> if (!mtk_smi_larb_ready(&larb_pdev->dev))
> return -EPROBE_DEFER;
> //====================
>
> The sequence is a little complex, If I don't explain clearly, please
> tell me.
> Any other suggestion about this? Thanks.
I'm still trying to find a different way to solve this. Unfortunately
I'm not able to reproduce this on my mt8173-evb...
...but: iommu_present checkes if the bus->iommu_ops is set. If we set
these option in the iommu after all components got initialized, we
should be fine. Or did I miss something?
Would you mind trying the patch below (against v4.9-rc1)?
If you know of any way to reproduce this issue, I would be happy to know.
I'm adding Joerg, the iommu maintainer, maybe he has an idea.
Thanks a lot,
Matthias
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index b12c12d..9249011 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -602,10 +602,12 @@ static int mtk_iommu_probe(struct platform_device
*pdev)
if (ret)
return ret;
- if (!iommu_present(&platform_bus_type))
+ ret = component_master_add_with_match(dev, &mtk_iommu_com_ops,
match);
+
+ if (ret && !iommu_present(&platform_bus_type))
bus_set_iommu(&platform_bus_type, &mtk_iommu_ops);
- return component_master_add_with_match(dev, &mtk_iommu_com_ops,
match);
+ return ret;
}
static int mtk_iommu_remove(struct platform_device *pdev)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH] [media] media: mtk-mdp: mark PM functions as __maybe_unused
From: Arnd Bergmann @ 2016-10-26 9:43 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Andrew-CT Chen, Minghsiu Tsai, Arnd Bergmann,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Houlong Wei, Hans Verkuil,
Matthias Brugger, linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-media-u79uwXL29TY76Z2rM5mHXA
A previous patch tried to fix a build error, but introduced another
warning:
drivers/media/platform/mtk-mdp/mtk_mdp_core.c:71:13: error: ‘mtk_mdp_clock_off’ defined but not used [-Werror=unused-function]
drivers/media/platform/mtk-mdp/mtk_mdp_core.c:62:13: error: ‘mtk_mdp_clock_on’ defined but not used [-Werror=unused-function]
This marks all the PM functions as __maybe_unused and removes
the #ifdef around them, as that will always do the right thing.
Fixes: 1b06fcf56aa6 ("[media] media: mtk-mdp: fix build error")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
index 40a229d8a1f5..d4c740263457 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
@@ -233,8 +233,7 @@ static int mtk_mdp_remove(struct platform_device *pdev)
return 0;
}
-#ifdef CONFIG_PM
-static int mtk_mdp_pm_suspend(struct device *dev)
+static int __maybe_unused mtk_mdp_pm_suspend(struct device *dev)
{
struct mtk_mdp_dev *mdp = dev_get_drvdata(dev);
@@ -243,7 +242,7 @@ static int mtk_mdp_pm_suspend(struct device *dev)
return 0;
}
-static int mtk_mdp_pm_resume(struct device *dev)
+static int __maybe_unused mtk_mdp_pm_resume(struct device *dev)
{
struct mtk_mdp_dev *mdp = dev_get_drvdata(dev);
@@ -251,10 +250,8 @@ static int mtk_mdp_pm_resume(struct device *dev)
return 0;
}
-#endif /* CONFIG_PM */
-#ifdef CONFIG_PM_SLEEP
-static int mtk_mdp_suspend(struct device *dev)
+static int __maybe_unused mtk_mdp_suspend(struct device *dev)
{
if (pm_runtime_suspended(dev))
return 0;
@@ -262,14 +259,13 @@ static int mtk_mdp_suspend(struct device *dev)
return mtk_mdp_pm_suspend(dev);
}
-static int mtk_mdp_resume(struct device *dev)
+static int __maybe_unused mtk_mdp_resume(struct device *dev)
{
if (pm_runtime_suspended(dev))
return 0;
return mtk_mdp_pm_resume(dev);
}
-#endif /* CONFIG_PM_SLEEP */
static const struct dev_pm_ops mtk_mdp_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(mtk_mdp_suspend, mtk_mdp_resume)
--
2.9.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related
* Re: [PATCH v3] drm/mediatek: fixed the calc method of data rate per lane
From: CK Hu @ 2016-10-26 9:37 UTC (permalink / raw)
To: Jitao Shi
Cc: Mark Rutland, stonea168, dri-devel, Andy Yan, Ajay Kumar,
Vincent Palatin, cawa.cheng, Russell King, devicetree, Pawel Moll,
Ian Campbell, Rob Herring, linux-mediatek, yingjoe.chen,
Matthias Brugger, eddie.huang, linux-arm-kernel, Rahul Sharma,
srv_heupstream, linux-kernel, Sascha Hauer, Kumar Gala
In-Reply-To: <1477472395-6451-1-git-send-email-jitao.shi@mediatek.com>
Hi, Jitao:
On Wed, 2016-10-26 at 16:59 +0800, Jitao Shi wrote:
> Tune dsi frame rate by pixel clock, dsi add some extra signal (i.e. Tlpx,
> Ths-prepare, Ths-zero, Ths-trail,Ths-exit) when enter and exit LP mode, this
> signal will cause h-time larger than normal and reduce FPS.
> Need to multiply a coefficient to offset the extra signal's effect.
> coefficient = ((htotal*bpp/lane_number)+Tlpx+Ths_prep+Ths_zero+Ths_trail+
> Ths_exit)/(htotal*bpp/lane_number))
>
One check-patch warning in commit message:
WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description
(prefer a maximum 75 chars per line)
#7:
Tune dsi frame rate by pixel clock, dsi add some extra signal (i.e.
Tlpx,
> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> ---
> Change since v2:
> - move phy timing back to dsi_phy_timconfig.
>
> Change since v1:
> - phy_timing2 and phy_timing3 refer clock cycle time.
> - define values of LPX HS_PRPR HS_ZERO HS_TRAIL TA_GO TA_SURE TA_GET DA_HS_EXIT
> ---
> drivers/gpu/drm/mediatek/mtk_dsi.c | 74 +++++++++++++++++++++++++-----------
> 1 file changed, 51 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 28b2044..8b3b38a 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -85,16 +85,16 @@
> #define LD0_WAKEUP_EN BIT(2)
>
> #define DSI_PHY_TIMECON0 0x110
> -#define LPX (0xff << 0)
> -#define HS_PRPR (0xff << 8)
> -#define HS_ZERO (0xff << 16)
> -#define HS_TRAIL (0xff << 24)
> +#define LPX (5 << 0)
> +#define HS_PRPR (6 << 8)
> +#define HS_ZERO (10 << 16)
> +#define HS_TRAIL (8 << 24)
>
> #define DSI_PHY_TIMECON1 0x114
> -#define TA_GO (0xff << 0)
> -#define TA_SURE (0xff << 8)
> -#define TA_GET (0xff << 16)
> -#define DA_HS_EXIT (0xff << 24)
> +#define TA_GO (20 << 0)
> +#define TA_SURE (7 << 8)
> +#define TA_GET (25 << 16)
> +#define DA_HS_EXIT (7 << 24)
>
> #define DSI_PHY_TIMECON2 0x118
> #define CONT_DET (0xff << 0)
> @@ -161,20 +161,18 @@ static void mtk_dsi_mask(struct mtk_dsi *dsi, u32 offset, u32 mask, u32 data)
> static void dsi_phy_timconfig(struct mtk_dsi *dsi)
> {
> u32 timcon0, timcon1, timcon2, timcon3;
> - unsigned int ui, cycle_time;
> - unsigned int lpx;
> + u32 ui, cycle_time;
>
> ui = 1000 / dsi->data_rate + 0x01;
> cycle_time = 8000 / dsi->data_rate + 0x01;
> - lpx = 5;
>
> - timcon0 = (8 << 24) | (0xa << 16) | (0x6 << 8) | lpx;
> - timcon1 = (7 << 24) | (5 * lpx << 16) | ((3 * lpx) / 2) << 8 |
> - (4 * lpx);
> + timcon0 = LPX | HS_PRPR | HS_ZERO | HS_TRAIL;
> + timcon1 = 4 * LPX | (3 * LPX / 2) << 8 | 5 * LPX << 16 | DA_HS_EXIT;
> timcon2 = ((NS_TO_CYCLE(0x64, cycle_time) + 0xa) << 24) |
> - (NS_TO_CYCLE(0x150, cycle_time) << 16);
> - timcon3 = (2 * lpx) << 16 | NS_TO_CYCLE(80 + 52 * ui, cycle_time) << 8 |
> - NS_TO_CYCLE(0x40, cycle_time);
> + (NS_TO_CYCLE(0x150, cycle_time) << 16);
I think this line may align to '(' in the right of '='.
> + timcon3 = (2 * LPX) << 16 |
> + NS_TO_CYCLE(80 + 52 * ui, cycle_time) << 8 |
> + NS_TO_CYCLE(0x40, cycle_time);
I think these two lines may align to '(' in the right of '='.
Regards,
CK
I.
>
> writel(timcon0, dsi->regs + DSI_PHY_TIMECON0);
> writel(timcon1, dsi->regs + DSI_PHY_TIMECON1);
> @@ -202,19 +200,49 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
> {
> struct device *dev = dsi->dev;
> int ret;
> + u64 bit_clock, total_bits;
> + u32 htotal, htotal_bits, bit_per_pixel, overhead_cycles, overhead_bits;
>
> if (++dsi->refcount != 1)
> return 0;
>
> + switch (dsi->format) {
> + case MIPI_DSI_FMT_RGB565:
> + bit_per_pixel = 16;
> + break;
> + case MIPI_DSI_FMT_RGB666_PACKED:
> + bit_per_pixel = 18;
> + break;
> + case MIPI_DSI_FMT_RGB666:
> + case MIPI_DSI_FMT_RGB888:
> + default:
> + bit_per_pixel = 24;
> + break;
> + }
> /**
> - * data_rate = (pixel_clock / 1000) * pixel_dipth * mipi_ratio;
> - * pixel_clock unit is Khz, data_rata unit is MHz, so need divide 1000.
> - * mipi_ratio is mipi clk coefficient for balance the pixel clk in mipi.
> - * we set mipi_ratio is 1.05.
> + * data_rate = (pixel_clock) * bit_per_pixel * mipi_ratio / lane_num;
> + * vm.pixelclock is Khz, data_rata unit is Hz, so need to multiply 1000
> + * mipi_ratio is (htotal * byte_per_pixel / lane_num + Tlpx + Ths_prep
> + * + Thstrail + Ths_exit + Ths_zero) /
> + * (htotal * byte_per_pixel /lane_number)
> */
> - dsi->data_rate = dsi->vm.pixelclock * 3 * 21 / (1 * 1000 * 10);
> + bit_clock = dsi->vm.pixelclock * 1000 * bit_per_pixel;
> + htotal = dsi->vm.hactive + dsi->vm.hback_porch + dsi->vm.hfront_porch +
> + dsi->vm.hsync_len;
> + htotal_bits = htotal * bit_per_pixel;
> +
> + /**
> + * overhead = lpx + hs_prepare + hs_zero + hs_trail + hs_exit
> + */
> + overhead_cycles = LPX + (HS_PRPR >> 8) + (HS_ZERO >> 16) +
> + (HS_TRAIL >> 24) + (DA_HS_EXIT >> 24);
> + overhead_bits = overhead_cycles * dsi->lanes * 8;
> + total_bits = htotal_bits + overhead_bits;
> +
> + dsi->data_rate = DIV_ROUND_UP_ULL(bit_clock * total_bits,
> + htotal_bits * dsi->lanes);
>
> - ret = clk_set_rate(dsi->hs_clk, dsi->data_rate * 1000000);
> + ret = clk_set_rate(dsi->hs_clk, dsi->data_rate);
> if (ret < 0) {
> dev_err(dev, "Failed to set data rate: %d\n", ret);
> goto err_refcount;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v2] drm/mediatek: fixed the calc method of data rate per lane
From: Jitao Shi @ 2016-10-26 9:07 UTC (permalink / raw)
To: CK Hu
Cc: Mark Rutland, stonea168, dri-devel, Andy Yan, Ajay Kumar,
Vincent Palatin, cawa.cheng, Russell King, devicetree, Pawel Moll,
Ian Campbell, Rob Herring, linux-mediatek, yingjoe.chen,
Matthias Brugger, eddie.huang, linux-arm-kernel, Rahul Sharma,
srv_heupstream, linux-kernel, Sascha Hauer, Kumar Gala
In-Reply-To: <1477464081.17405.2.camel@mtksdaap41>
On Wed, 2016-10-26 at 14:41 +0800, CK Hu wrote:
> Hi, Jitao:
>
> On Tue, 2016-10-25 at 13:40 +0800, Jitao Shi wrote:
> > Tune dsi frame rate by pixel clock, dsi add some extra signal (i.e. Tlpx,
> > Ths-prepare, Ths-zero, Ths-trail,Ths-exit) when enter and exit LP mode, this
> > signal will cause h-time larger than normal and reduce FPS.
> > Need to multiply a coefficient to offset the extra signal's effect.
> > coefficient = ((htotal*bpp/lane_number)+Tlpx+Ths_prep+Ths_zero+Ths_trail+
> > Ths_exit)/(htotal*bpp/lane_number))
> >
> > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> > ---
> > Change since v1:
> > - phy_timing2 and phy_timing3 refer clock cycle time.
> > - define values of LPX HS_PRPR HS_ZERO HS_TRAIL TA_GO TA_SURE TA_GET DA_HS_EXIT
> > ---
> > drivers/gpu/drm/mediatek/mtk_dsi.c | 103 +++++++++++++++++++++++-------------
> > 1 file changed, 67 insertions(+), 36 deletions(-)
> >
>
> [snip...]
>
> >
> > -static void dsi_phy_timconfig(struct mtk_dsi *dsi)
> > +static void dsi_phy_timconfig(struct mtk_dsi *dsi, u32 phy_timing0,
> > + u32 phy_timing1, u32 phy_timing2,
> > + u32 phy_timing3)
> > {
> > - u32 timcon0, timcon1, timcon2, timcon3;
> > - unsigned int ui, cycle_time;
> > - unsigned int lpx;
> > -
> > - ui = 1000 / dsi->data_rate + 0x01;
> > - cycle_time = 8000 / dsi->data_rate + 0x01;
> > - lpx = 5;
> > -
> > - timcon0 = (8 << 24) | (0xa << 16) | (0x6 << 8) | lpx;
> > - timcon1 = (7 << 24) | (5 * lpx << 16) | ((3 * lpx) / 2) << 8 |
> > - (4 * lpx);
> > - timcon2 = ((NS_TO_CYCLE(0x64, cycle_time) + 0xa) << 24) |
> > - (NS_TO_CYCLE(0x150, cycle_time) << 16);
> > - timcon3 = (2 * lpx) << 16 | NS_TO_CYCLE(80 + 52 * ui, cycle_time) << 8 |
> > - NS_TO_CYCLE(0x40, cycle_time);
> > -
> > - writel(timcon0, dsi->regs + DSI_PHY_TIMECON0);
> > - writel(timcon1, dsi->regs + DSI_PHY_TIMECON1);
> > - writel(timcon2, dsi->regs + DSI_PHY_TIMECON2);
> > - writel(timcon3, dsi->regs + DSI_PHY_TIMECON3);
>
> Why do you move these calculation to mtk_dsi_poweron()? You can keep
> calculation here and just do some modification.
>
> Regards,
> CK
Thanks for your review. I'll fix it in next patchset.
Best Regards
jitao
>
> > + writel(phy_timing0, dsi->regs + DSI_PHY_TIMECON0);
> > + writel(phy_timing1, dsi->regs + DSI_PHY_TIMECON1);
> > + writel(phy_timing2, dsi->regs + DSI_PHY_TIMECON2);
> > + writel(phy_timing3, dsi->regs + DSI_PHY_TIMECON3);
> > }
> >
> > static void mtk_dsi_enable(struct mtk_dsi *dsi)
> > @@ -202,19 +188,51 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
> > {
> > struct device *dev = dsi->dev;
> > int ret;
> > + u64 bit_clock, total_bits;
> > + u32 htotal, htotal_bits, bit_per_pixel, overhead_cycles, overhead_bits;
> > + u32 phy_timing0, phy_timing1, phy_timing2, phy_timing3;
> > + u32 ui, cycle_time;
> >
> > if (++dsi->refcount != 1)
> > return 0;
> >
> > + switch (dsi->format) {
> > + case MIPI_DSI_FMT_RGB565:
> > + bit_per_pixel = 16;
> > + break;
> > + case MIPI_DSI_FMT_RGB666_PACKED:
> > + bit_per_pixel = 18;
> > + break;
> > + case MIPI_DSI_FMT_RGB666:
> > + case MIPI_DSI_FMT_RGB888:
> > + default:
> > + bit_per_pixel = 24;
> > + break;
> > + }
> > + /**
> > + * data_rate = (pixel_clock) * bit_per_pixel * mipi_ratio / lane_num;
> > + * vm.pixelclock is Khz, data_rata unit is Hz, so need to multiply 1000
> > + * mipi_ratio is (htotal * byte_per_pixel / lane_num + Tlpx + Ths_prep
> > + * + Thstrail + Ths_exit + Ths_zero) /
> > + * (htotal * byte_per_pixel /lane_number)
> > + */
> > + bit_clock = dsi->vm.pixelclock * 1000 * bit_per_pixel;
> > + htotal = dsi->vm.hactive + dsi->vm.hback_porch + dsi->vm.hfront_porch +
> > + dsi->vm.hsync_len;
> > + htotal_bits = htotal * bit_per_pixel;
> > +
> > /**
> > - * data_rate = (pixel_clock / 1000) * pixel_dipth * mipi_ratio;
> > - * pixel_clock unit is Khz, data_rata unit is MHz, so need divide 1000.
> > - * mipi_ratio is mipi clk coefficient for balance the pixel clk in mipi.
> > - * we set mipi_ratio is 1.05.
> > + * overhead = lpx + hs_prepare + hs_zero + hs_trail + hs_exit
> > */
> > - dsi->data_rate = dsi->vm.pixelclock * 3 * 21 / (1 * 1000 * 10);
> > + overhead_cycles = LPX + (HS_PRPR >> 8) + (HS_ZERO >> 16) +
> > + (HS_TRAIL >> 24) + (DA_HS_EXIT >> 24);
> > + overhead_bits = overhead_cycles * dsi->lanes * 8;
> > + total_bits = htotal_bits + overhead_bits;
> >
> > - ret = clk_set_rate(dsi->hs_clk, dsi->data_rate * 1000000);
> > + dsi->data_rate = DIV_ROUND_UP_ULL(bit_clock * total_bits,
> > + htotal_bits * dsi->lanes);
> > +
> > + ret = clk_set_rate(dsi->hs_clk, dsi->data_rate);
> > if (ret < 0) {
> > dev_err(dev, "Failed to set data rate: %d\n", ret);
> > goto err_refcount;
> > @@ -236,7 +254,20 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
> >
> > mtk_dsi_enable(dsi);
> > mtk_dsi_reset(dsi);
> > - dsi_phy_timconfig(dsi);
> > +
> > + ui = 1000 / dsi->data_rate + 0x01;
> > + cycle_time = 8000 / dsi->data_rate + 0x01;
> > +
> > + phy_timing0 = LPX | HS_PRPR | HS_ZERO | HS_TRAIL;
> > + phy_timing1 = TA_GO | TA_SURE | TA_GET | DA_HS_EXIT;
> > + phy_timing2 = ((NS_TO_CYCLE(0x64, cycle_time) + 0xa) << 24) |
> > + (NS_TO_CYCLE(0x150, cycle_time) << 16);
> > + phy_timing3 = (2 * LPX) << 16 |
> > + NS_TO_CYCLE(80 + 52 * ui, cycle_time) << 8 |
> > + NS_TO_CYCLE(0x40, cycle_time);
> > +
> > + dsi_phy_timconfig(dsi, phy_timing0, phy_timing1, phy_timing2,
> > + phy_timing3);
> >
> > return 0;
> >
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH v3] drm/mediatek: fixed the calc method of data rate per lane
From: Jitao Shi @ 2016-10-26 8:59 UTC (permalink / raw)
To: Philipp Zabel, ck.hu, Matthias Brugger
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Jitao Shi, Ajay Kumar, Inki Dae, Rahul Sharma, Sean Paul,
Vincent Palatin, Andy Yan, Russell King, devicetree, linux-kernel,
dri-devel, linux-arm-kernel, linux-mediatek, srv_heupstream,
Sascha Hauer, yingjoe.chen
Tune dsi frame rate by pixel clock, dsi add some extra signal (i.e. Tlpx,
Ths-prepare, Ths-zero, Ths-trail,Ths-exit) when enter and exit LP mode, this
signal will cause h-time larger than normal and reduce FPS.
Need to multiply a coefficient to offset the extra signal's effect.
coefficient = ((htotal*bpp/lane_number)+Tlpx+Ths_prep+Ths_zero+Ths_trail+
Ths_exit)/(htotal*bpp/lane_number))
Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
---
Change since v2:
- move phy timing back to dsi_phy_timconfig.
Change since v1:
- phy_timing2 and phy_timing3 refer clock cycle time.
- define values of LPX HS_PRPR HS_ZERO HS_TRAIL TA_GO TA_SURE TA_GET DA_HS_EXIT
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 74 +++++++++++++++++++++++++-----------
1 file changed, 51 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 28b2044..8b3b38a 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -85,16 +85,16 @@
#define LD0_WAKEUP_EN BIT(2)
#define DSI_PHY_TIMECON0 0x110
-#define LPX (0xff << 0)
-#define HS_PRPR (0xff << 8)
-#define HS_ZERO (0xff << 16)
-#define HS_TRAIL (0xff << 24)
+#define LPX (5 << 0)
+#define HS_PRPR (6 << 8)
+#define HS_ZERO (10 << 16)
+#define HS_TRAIL (8 << 24)
#define DSI_PHY_TIMECON1 0x114
-#define TA_GO (0xff << 0)
-#define TA_SURE (0xff << 8)
-#define TA_GET (0xff << 16)
-#define DA_HS_EXIT (0xff << 24)
+#define TA_GO (20 << 0)
+#define TA_SURE (7 << 8)
+#define TA_GET (25 << 16)
+#define DA_HS_EXIT (7 << 24)
#define DSI_PHY_TIMECON2 0x118
#define CONT_DET (0xff << 0)
@@ -161,20 +161,18 @@ static void mtk_dsi_mask(struct mtk_dsi *dsi, u32 offset, u32 mask, u32 data)
static void dsi_phy_timconfig(struct mtk_dsi *dsi)
{
u32 timcon0, timcon1, timcon2, timcon3;
- unsigned int ui, cycle_time;
- unsigned int lpx;
+ u32 ui, cycle_time;
ui = 1000 / dsi->data_rate + 0x01;
cycle_time = 8000 / dsi->data_rate + 0x01;
- lpx = 5;
- timcon0 = (8 << 24) | (0xa << 16) | (0x6 << 8) | lpx;
- timcon1 = (7 << 24) | (5 * lpx << 16) | ((3 * lpx) / 2) << 8 |
- (4 * lpx);
+ timcon0 = LPX | HS_PRPR | HS_ZERO | HS_TRAIL;
+ timcon1 = 4 * LPX | (3 * LPX / 2) << 8 | 5 * LPX << 16 | DA_HS_EXIT;
timcon2 = ((NS_TO_CYCLE(0x64, cycle_time) + 0xa) << 24) |
- (NS_TO_CYCLE(0x150, cycle_time) << 16);
- timcon3 = (2 * lpx) << 16 | NS_TO_CYCLE(80 + 52 * ui, cycle_time) << 8 |
- NS_TO_CYCLE(0x40, cycle_time);
+ (NS_TO_CYCLE(0x150, cycle_time) << 16);
+ timcon3 = (2 * LPX) << 16 |
+ NS_TO_CYCLE(80 + 52 * ui, cycle_time) << 8 |
+ NS_TO_CYCLE(0x40, cycle_time);
writel(timcon0, dsi->regs + DSI_PHY_TIMECON0);
writel(timcon1, dsi->regs + DSI_PHY_TIMECON1);
@@ -202,19 +200,49 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
{
struct device *dev = dsi->dev;
int ret;
+ u64 bit_clock, total_bits;
+ u32 htotal, htotal_bits, bit_per_pixel, overhead_cycles, overhead_bits;
if (++dsi->refcount != 1)
return 0;
+ switch (dsi->format) {
+ case MIPI_DSI_FMT_RGB565:
+ bit_per_pixel = 16;
+ break;
+ case MIPI_DSI_FMT_RGB666_PACKED:
+ bit_per_pixel = 18;
+ break;
+ case MIPI_DSI_FMT_RGB666:
+ case MIPI_DSI_FMT_RGB888:
+ default:
+ bit_per_pixel = 24;
+ break;
+ }
/**
- * data_rate = (pixel_clock / 1000) * pixel_dipth * mipi_ratio;
- * pixel_clock unit is Khz, data_rata unit is MHz, so need divide 1000.
- * mipi_ratio is mipi clk coefficient for balance the pixel clk in mipi.
- * we set mipi_ratio is 1.05.
+ * data_rate = (pixel_clock) * bit_per_pixel * mipi_ratio / lane_num;
+ * vm.pixelclock is Khz, data_rata unit is Hz, so need to multiply 1000
+ * mipi_ratio is (htotal * byte_per_pixel / lane_num + Tlpx + Ths_prep
+ * + Thstrail + Ths_exit + Ths_zero) /
+ * (htotal * byte_per_pixel /lane_number)
*/
- dsi->data_rate = dsi->vm.pixelclock * 3 * 21 / (1 * 1000 * 10);
+ bit_clock = dsi->vm.pixelclock * 1000 * bit_per_pixel;
+ htotal = dsi->vm.hactive + dsi->vm.hback_porch + dsi->vm.hfront_porch +
+ dsi->vm.hsync_len;
+ htotal_bits = htotal * bit_per_pixel;
+
+ /**
+ * overhead = lpx + hs_prepare + hs_zero + hs_trail + hs_exit
+ */
+ overhead_cycles = LPX + (HS_PRPR >> 8) + (HS_ZERO >> 16) +
+ (HS_TRAIL >> 24) + (DA_HS_EXIT >> 24);
+ overhead_bits = overhead_cycles * dsi->lanes * 8;
+ total_bits = htotal_bits + overhead_bits;
+
+ dsi->data_rate = DIV_ROUND_UP_ULL(bit_clock * total_bits,
+ htotal_bits * dsi->lanes);
- ret = clk_set_rate(dsi->hs_clk, dsi->data_rate * 1000000);
+ ret = clk_set_rate(dsi->hs_clk, dsi->data_rate);
if (ret < 0) {
dev_err(dev, "Failed to set data rate: %d\n", ret);
goto err_refcount;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v2] drm/mediatek: fixed the calc method of data rate per lane
From: CK Hu @ 2016-10-26 6:41 UTC (permalink / raw)
To: Jitao Shi
Cc: Mark Rutland, stonea168, dri-devel, Andy Yan, Ajay Kumar,
Vincent Palatin, cawa.cheng, Russell King, devicetree, Pawel Moll,
Ian Campbell, Rob Herring, linux-mediatek, yingjoe.chen,
Matthias Brugger, eddie.huang, linux-arm-kernel, Rahul Sharma,
srv_heupstream, linux-kernel, Sascha Hauer, Kumar Gala
In-Reply-To: <1477374045-2837-1-git-send-email-jitao.shi@mediatek.com>
Hi, Jitao:
On Tue, 2016-10-25 at 13:40 +0800, Jitao Shi wrote:
> Tune dsi frame rate by pixel clock, dsi add some extra signal (i.e. Tlpx,
> Ths-prepare, Ths-zero, Ths-trail,Ths-exit) when enter and exit LP mode, this
> signal will cause h-time larger than normal and reduce FPS.
> Need to multiply a coefficient to offset the extra signal's effect.
> coefficient = ((htotal*bpp/lane_number)+Tlpx+Ths_prep+Ths_zero+Ths_trail+
> Ths_exit)/(htotal*bpp/lane_number))
>
> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> ---
> Change since v1:
> - phy_timing2 and phy_timing3 refer clock cycle time.
> - define values of LPX HS_PRPR HS_ZERO HS_TRAIL TA_GO TA_SURE TA_GET DA_HS_EXIT
> ---
> drivers/gpu/drm/mediatek/mtk_dsi.c | 103 +++++++++++++++++++++++-------------
> 1 file changed, 67 insertions(+), 36 deletions(-)
>
[snip...]
>
> -static void dsi_phy_timconfig(struct mtk_dsi *dsi)
> +static void dsi_phy_timconfig(struct mtk_dsi *dsi, u32 phy_timing0,
> + u32 phy_timing1, u32 phy_timing2,
> + u32 phy_timing3)
> {
> - u32 timcon0, timcon1, timcon2, timcon3;
> - unsigned int ui, cycle_time;
> - unsigned int lpx;
> -
> - ui = 1000 / dsi->data_rate + 0x01;
> - cycle_time = 8000 / dsi->data_rate + 0x01;
> - lpx = 5;
> -
> - timcon0 = (8 << 24) | (0xa << 16) | (0x6 << 8) | lpx;
> - timcon1 = (7 << 24) | (5 * lpx << 16) | ((3 * lpx) / 2) << 8 |
> - (4 * lpx);
> - timcon2 = ((NS_TO_CYCLE(0x64, cycle_time) + 0xa) << 24) |
> - (NS_TO_CYCLE(0x150, cycle_time) << 16);
> - timcon3 = (2 * lpx) << 16 | NS_TO_CYCLE(80 + 52 * ui, cycle_time) << 8 |
> - NS_TO_CYCLE(0x40, cycle_time);
> -
> - writel(timcon0, dsi->regs + DSI_PHY_TIMECON0);
> - writel(timcon1, dsi->regs + DSI_PHY_TIMECON1);
> - writel(timcon2, dsi->regs + DSI_PHY_TIMECON2);
> - writel(timcon3, dsi->regs + DSI_PHY_TIMECON3);
Why do you move these calculation to mtk_dsi_poweron()? You can keep
calculation here and just do some modification.
Regards,
CK
> + writel(phy_timing0, dsi->regs + DSI_PHY_TIMECON0);
> + writel(phy_timing1, dsi->regs + DSI_PHY_TIMECON1);
> + writel(phy_timing2, dsi->regs + DSI_PHY_TIMECON2);
> + writel(phy_timing3, dsi->regs + DSI_PHY_TIMECON3);
> }
>
> static void mtk_dsi_enable(struct mtk_dsi *dsi)
> @@ -202,19 +188,51 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
> {
> struct device *dev = dsi->dev;
> int ret;
> + u64 bit_clock, total_bits;
> + u32 htotal, htotal_bits, bit_per_pixel, overhead_cycles, overhead_bits;
> + u32 phy_timing0, phy_timing1, phy_timing2, phy_timing3;
> + u32 ui, cycle_time;
>
> if (++dsi->refcount != 1)
> return 0;
>
> + switch (dsi->format) {
> + case MIPI_DSI_FMT_RGB565:
> + bit_per_pixel = 16;
> + break;
> + case MIPI_DSI_FMT_RGB666_PACKED:
> + bit_per_pixel = 18;
> + break;
> + case MIPI_DSI_FMT_RGB666:
> + case MIPI_DSI_FMT_RGB888:
> + default:
> + bit_per_pixel = 24;
> + break;
> + }
> + /**
> + * data_rate = (pixel_clock) * bit_per_pixel * mipi_ratio / lane_num;
> + * vm.pixelclock is Khz, data_rata unit is Hz, so need to multiply 1000
> + * mipi_ratio is (htotal * byte_per_pixel / lane_num + Tlpx + Ths_prep
> + * + Thstrail + Ths_exit + Ths_zero) /
> + * (htotal * byte_per_pixel /lane_number)
> + */
> + bit_clock = dsi->vm.pixelclock * 1000 * bit_per_pixel;
> + htotal = dsi->vm.hactive + dsi->vm.hback_porch + dsi->vm.hfront_porch +
> + dsi->vm.hsync_len;
> + htotal_bits = htotal * bit_per_pixel;
> +
> /**
> - * data_rate = (pixel_clock / 1000) * pixel_dipth * mipi_ratio;
> - * pixel_clock unit is Khz, data_rata unit is MHz, so need divide 1000.
> - * mipi_ratio is mipi clk coefficient for balance the pixel clk in mipi.
> - * we set mipi_ratio is 1.05.
> + * overhead = lpx + hs_prepare + hs_zero + hs_trail + hs_exit
> */
> - dsi->data_rate = dsi->vm.pixelclock * 3 * 21 / (1 * 1000 * 10);
> + overhead_cycles = LPX + (HS_PRPR >> 8) + (HS_ZERO >> 16) +
> + (HS_TRAIL >> 24) + (DA_HS_EXIT >> 24);
> + overhead_bits = overhead_cycles * dsi->lanes * 8;
> + total_bits = htotal_bits + overhead_bits;
>
> - ret = clk_set_rate(dsi->hs_clk, dsi->data_rate * 1000000);
> + dsi->data_rate = DIV_ROUND_UP_ULL(bit_clock * total_bits,
> + htotal_bits * dsi->lanes);
> +
> + ret = clk_set_rate(dsi->hs_clk, dsi->data_rate);
> if (ret < 0) {
> dev_err(dev, "Failed to set data rate: %d\n", ret);
> goto err_refcount;
> @@ -236,7 +254,20 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
>
> mtk_dsi_enable(dsi);
> mtk_dsi_reset(dsi);
> - dsi_phy_timconfig(dsi);
> +
> + ui = 1000 / dsi->data_rate + 0x01;
> + cycle_time = 8000 / dsi->data_rate + 0x01;
> +
> + phy_timing0 = LPX | HS_PRPR | HS_ZERO | HS_TRAIL;
> + phy_timing1 = TA_GO | TA_SURE | TA_GET | DA_HS_EXIT;
> + phy_timing2 = ((NS_TO_CYCLE(0x64, cycle_time) + 0xa) << 24) |
> + (NS_TO_CYCLE(0x150, cycle_time) << 16);
> + phy_timing3 = (2 * LPX) << 16 |
> + NS_TO_CYCLE(80 + 52 * ui, cycle_time) << 8 |
> + NS_TO_CYCLE(0x40, cycle_time);
> +
> + dsi_phy_timconfig(dsi, phy_timing0, phy_timing1, phy_timing2,
> + phy_timing3);
>
> return 0;
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH] drm: convert DT component matching to component_match_add_release()
From: Sean Paul @ 2016-10-25 16:44 UTC (permalink / raw)
To: Russell King
Cc: Liviu Dudau, dri-devel, Chen-Yu Tsai, Xinliang Liu,
linux-rockchip, Tomi Valkeinen, Mali DP Maintainers, Chen Feng,
linux-arm-msm, Jyri Sarha, linux-mediatek, Matthias Brugger,
Vincent Abriou, Linux ARM Kernel, Maxime Ripard, freedreno
In-Reply-To: <E1bwo6l-0005Io-Q1@rmk-PC.armlinux.org.uk>
On Wed, Oct 19, 2016 at 6:28 AM, Russell King
<rmk+kernel@armlinux.org.uk> wrote:
> Convert DT component matching to use component_match_add_release().
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Applied to drm-misc, thanks!
Sean
> ---
> Can we please get this patch from May merged into the drm-misc or
> whatever trees so that we don't end up with conflicts? I've no idea
> who looks after drm-misc, as they have _still_ failed to add
> themselves to MAINTAINERS.
>
> drivers/gpu/drm/arm/hdlcd_drv.c | 3 ++-
> drivers/gpu/drm/arm/malidp_drv.c | 4 +++-
> drivers/gpu/drm/armada/armada_drv.c | 2 +-
> drivers/gpu/drm/drm_of.c | 28 +++++++++++++++++++++++--
> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 5 +++--
> drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 7 ++++---
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 +++-
> drivers/gpu/drm/msm/msm_drv.c | 12 ++++++-----
> drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 6 ++++--
> drivers/gpu/drm/sti/sti_drv.c | 5 +++--
> drivers/gpu/drm/sun4i/sun4i_drv.c | 3 ++-
> drivers/gpu/drm/tilcdc/tilcdc_external.c | 4 +++-
> include/drm/drm_of.h | 12 +++++++++++
> 13 files changed, 73 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
> index fb6a418ce6be..6477d1a65266 100644
> --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> @@ -453,7 +453,8 @@ static int hdlcd_probe(struct platform_device *pdev)
> return -EAGAIN;
> }
>
> - component_match_add(&pdev->dev, &match, compare_dev, port);
> + drm_of_component_match_add(&pdev->dev, &match, compare_dev, port);
> + of_node_put(port);
>
> return component_master_add_with_match(&pdev->dev, &hdlcd_master_ops,
> match);
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 9280358b8f15..9f4739452a25 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -493,7 +493,9 @@ static int malidp_platform_probe(struct platform_device *pdev)
> return -EAGAIN;
> }
>
> - component_match_add(&pdev->dev, &match, malidp_compare_dev, port);
> + drm_of_component_match_add(&pdev->dev, &match, malidp_compare_dev,
> + port);
> + of_node_put(port);
> return component_master_add_with_match(&pdev->dev, &malidp_master_ops,
> match);
> }
> diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
> index 1e0e68f608e4..94e46da9a758 100644
> --- a/drivers/gpu/drm/armada/armada_drv.c
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -254,7 +254,7 @@ static void armada_add_endpoints(struct device *dev,
> continue;
> }
>
> - component_match_add(dev, match, compare_of, remote);
> + drm_of_component_match_add(dev, match, compare_of, remote);
> of_node_put(remote);
> }
> }
> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> index bc98bb94264d..47848ed8ca48 100644
> --- a/drivers/gpu/drm/drm_of.c
> +++ b/drivers/gpu/drm/drm_of.c
> @@ -6,6 +6,11 @@
> #include <drm/drm_crtc.h>
> #include <drm/drm_of.h>
>
> +static void drm_release_of(struct device *dev, void *data)
> +{
> + of_node_put(data);
> +}
> +
> /**
> * drm_crtc_port_mask - find the mask of a registered CRTC by port OF node
> * @dev: DRM device
> @@ -64,6 +69,24 @@ uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
> EXPORT_SYMBOL(drm_of_find_possible_crtcs);
>
> /**
> + * drm_of_component_match_add - Add a component helper OF node match rule
> + * @master: master device
> + * @matchptr: component match pointer
> + * @compare: compare function used for matching component
> + * @node: of_node
> + */
> +void drm_of_component_match_add(struct device *master,
> + struct component_match **matchptr,
> + int (*compare)(struct device *, void *),
> + struct device_node *node)
> +{
> + of_node_get(node);
> + component_match_add_release(master, matchptr, drm_release_of,
> + compare, node);
> +}
> +EXPORT_SYMBOL_GPL(drm_of_component_match_add);
> +
> +/**
> * drm_of_component_probe - Generic probe function for a component based master
> * @dev: master device containing the OF node
> * @compare_of: compare function used for matching components
> @@ -101,7 +124,7 @@ int drm_of_component_probe(struct device *dev,
> continue;
> }
>
> - component_match_add(dev, &match, compare_of, port);
> + drm_of_component_match_add(dev, &match, compare_of, port);
> of_node_put(port);
> }
>
> @@ -140,7 +163,8 @@ int drm_of_component_probe(struct device *dev,
> continue;
> }
>
> - component_match_add(dev, &match, compare_of, remote);
> + drm_of_component_match_add(dev, &match, compare_of,
> + remote);
> of_node_put(remote);
> }
> of_node_put(port);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index aa687669e22b..0dee6acbd880 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -16,6 +16,7 @@
>
> #include <linux/component.h>
> #include <linux/of_platform.h>
> +#include <drm/drm_of.h>
>
> #include "etnaviv_drv.h"
> #include "etnaviv_gpu.h"
> @@ -629,8 +630,8 @@ static int etnaviv_pdev_probe(struct platform_device *pdev)
> if (!core_node)
> break;
>
> - component_match_add(&pdev->dev, &match, compare_of,
> - core_node);
> + drm_of_component_match_add(&pdev->dev, &match,
> + compare_of, core_node);
> of_node_put(core_node);
> }
> } else if (dev->platform_data) {
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> index 90377a609c98..e88fde18c946 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> @@ -24,6 +24,7 @@
> #include <drm/drm_fb_cma_helper.h>
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_crtc_helper.h>
> +#include <drm/drm_of.h>
>
> #include "kirin_drm_drv.h"
>
> @@ -260,14 +261,13 @@ static struct device_node *kirin_get_remote_node(struct device_node *np)
> DRM_ERROR("no valid endpoint node\n");
> return ERR_PTR(-ENODEV);
> }
> - of_node_put(endpoint);
>
> remote = of_graph_get_remote_port_parent(endpoint);
> + of_node_put(endpoint);
> if (!remote) {
> DRM_ERROR("no valid remote node\n");
> return ERR_PTR(-ENODEV);
> }
> - of_node_put(remote);
>
> if (!of_device_is_available(remote)) {
> DRM_ERROR("not available for remote node\n");
> @@ -294,7 +294,8 @@ static int kirin_drm_platform_probe(struct platform_device *pdev)
> if (IS_ERR(remote))
> return PTR_ERR(remote);
>
> - component_match_add(dev, &match, compare_of, remote);
> + drm_of_component_match_add(dev, &match, compare_of, remote);
> + of_node_put(remote);
>
> return component_master_add_with_match(dev, &kirin_drm_ops, match);
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index cf83f6507ec8..9c5430fb82a2 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -18,6 +18,7 @@
> #include <drm/drm_crtc_helper.h>
> #include <drm/drm_gem.h>
> #include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_of.h>
> #include <linux/component.h>
> #include <linux/iommu.h>
> #include <linux/of_address.h>
> @@ -415,7 +416,8 @@ static int mtk_drm_probe(struct platform_device *pdev)
> comp_type == MTK_DPI) {
> dev_info(dev, "Adding component match for %s\n",
> node->full_name);
> - component_match_add(dev, &match, compare_of, node);
> + drm_of_component_match_add(dev, &match, compare_of,
> + node);
> } else {
> struct mtk_ddp_comp *comp;
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index fb5c0b0a7594..84d38eaea585 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -15,6 +15,8 @@
> * this program. If not, see <http://www.gnu.org/licenses/>.
> */
>
> +#include <drm/drm_of.h>
> +
> #include "msm_drv.h"
> #include "msm_debugfs.h"
> #include "msm_fence.h"
> @@ -919,8 +921,8 @@ static int add_components_mdp(struct device *mdp_dev,
> continue;
> }
>
> - component_match_add(master_dev, matchptr, compare_of, intf);
> -
> + drm_of_component_match_add(master_dev, matchptr, compare_of,
> + intf);
> of_node_put(intf);
> of_node_put(ep_node);
> }
> @@ -962,8 +964,8 @@ static int add_display_components(struct device *dev,
> put_device(mdp_dev);
>
> /* add the MDP component itself */
> - component_match_add(dev, matchptr, compare_of,
> - mdp_dev->of_node);
> + drm_of_component_match_add(dev, matchptr, compare_of,
> + mdp_dev->of_node);
> } else {
> /* MDP4 */
> mdp_dev = dev;
> @@ -996,7 +998,7 @@ static int add_gpu_components(struct device *dev,
> if (!np)
> return 0;
>
> - component_match_add(dev, matchptr, compare_of, np);
> + drm_of_component_match_add(dev, matchptr, compare_of, np);
>
> of_node_put(np);
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index 8c8cbe837e61..6fe161192bb4 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -20,6 +20,7 @@
> #include <drm/drm_crtc_helper.h>
> #include <drm/drm_fb_helper.h>
> #include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_of.h>
> #include <linux/dma-mapping.h>
> #include <linux/pm_runtime.h>
> #include <linux/module.h>
> @@ -388,7 +389,7 @@ static void rockchip_add_endpoints(struct device *dev,
> continue;
> }
>
> - component_match_add(dev, match, compare_of, remote);
> + drm_of_component_match_add(dev, match, compare_of, remote);
> of_node_put(remote);
> }
> }
> @@ -437,7 +438,8 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
> }
>
> of_node_put(iommu);
> - component_match_add(dev, &match, compare_of, port->parent);
> + drm_of_component_match_add(dev, &match, compare_of,
> + port->parent);
> of_node_put(port);
> }
>
> diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
> index 2784919a7366..5e819876e642 100644
> --- a/drivers/gpu/drm/sti/sti_drv.c
> +++ b/drivers/gpu/drm/sti/sti_drv.c
> @@ -17,6 +17,7 @@
> #include <drm/drm_crtc_helper.h>
> #include <drm/drm_gem_cma_helper.h>
> #include <drm/drm_fb_cma_helper.h>
> +#include <drm/drm_of.h>
>
> #include "sti_crtc.h"
> #include "sti_drv.h"
> @@ -423,8 +424,8 @@ static int sti_platform_probe(struct platform_device *pdev)
> child_np = of_get_next_available_child(node, NULL);
>
> while (child_np) {
> - component_match_add(dev, &match, compare_of, child_np);
> - of_node_put(child_np);
> + drm_of_component_match_add(dev, &match, compare_of,
> + child_np);
> child_np = of_get_next_available_child(node, child_np);
> }
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
> index 0da9862ad8ed..b3c4ad605e81 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> @@ -18,6 +18,7 @@
> #include <drm/drm_fb_cma_helper.h>
> #include <drm/drm_gem_cma_helper.h>
> #include <drm/drm_fb_helper.h>
> +#include <drm/drm_of.h>
>
> #include "sun4i_crtc.h"
> #include "sun4i_drv.h"
> @@ -239,7 +240,7 @@ static int sun4i_drv_add_endpoints(struct device *dev,
> /* Add current component */
> DRM_DEBUG_DRIVER("Adding component %s\n",
> of_node_full_name(node));
> - component_match_add(dev, match, compare_of, node);
> + drm_of_component_match_add(dev, match, compare_of, node);
> count++;
> }
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> index 68e895021005..06a4c584f3cb 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_external.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> @@ -10,6 +10,7 @@
>
> #include <linux/component.h>
> #include <linux/of_graph.h>
> +#include <drm/drm_of.h>
>
> #include "tilcdc_drv.h"
> #include "tilcdc_external.h"
> @@ -160,7 +161,8 @@ int tilcdc_get_external_components(struct device *dev,
>
> dev_dbg(dev, "Subdevice node '%s' found\n", node->name);
> if (match)
> - component_match_add(dev, match, dev_match_of, node);
> + drm_of_component_match_add(dev, match, dev_match_of,
> + node);
> of_node_put(node);
> count++;
> }
> diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h
> index 3fd87b386ed7..d6b4c5587bbe 100644
> --- a/include/drm/drm_of.h
> +++ b/include/drm/drm_of.h
> @@ -4,6 +4,7 @@
> #include <linux/of_graph.h>
>
> struct component_master_ops;
> +struct component_match;
> struct device;
> struct drm_device;
> struct drm_encoder;
> @@ -12,6 +13,10 @@ struct device_node;
> #ifdef CONFIG_OF
> extern uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
> struct device_node *port);
> +extern void drm_of_component_match_add(struct device *master,
> + struct component_match **matchptr,
> + int (*compare)(struct device *, void *),
> + struct device_node *node);
> extern int drm_of_component_probe(struct device *dev,
> int (*compare_of)(struct device *, void *),
> const struct component_master_ops *m_ops);
> @@ -25,6 +30,13 @@ static inline uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
> return 0;
> }
>
> +static void drm_of_component_match_add(struct device *master,
> + struct component_match **matchptr,
> + int (*compare)(struct device *, void *),
> + struct device_node *node)
> +{
> +}
> +
> static inline int
> drm_of_component_probe(struct device *dev,
> int (*compare_of)(struct device *, void *),
> --
> 2.1.0
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v9 1/4] soc: mediatek: Refine scpsys to support multiple platform
From: Matthias Brugger @ 2016-10-25 14:04 UTC (permalink / raw)
To: James Liao, Sascha Hauer
Cc: Rob Herring, Kevin Hilman, Daniel Kurtz,
srv_heupstream-NuS5LvNUpcJWk0Htik3J/w,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1476953798-23263-2-git-send-email-jamesjj.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
Hi James,
On 10/20/2016 10:56 AM, James Liao wrote:
> -static int scpsys_probe(struct platform_device *pdev)
> +static void init_clks(struct platform_device *pdev, struct clk *clk[CLK_MAX])
I prefer struct clk **clk.
> +{
> + int i;
> +
> + for (i = CLK_NONE + 1; i < CLK_MAX; i++)
> + clk[i] = devm_clk_get(&pdev->dev, clk_names[i]);
> +}
> +
> +static struct scp *init_scp(struct platform_device *pdev,
> + const struct scp_domain_data *scp_domain_data, int num)
> {
> struct genpd_onecell_data *pd_data;
> struct resource *res;
> - int i, j, ret;
> + int i, j;
> struct scp *scp;
> - struct clk *clk[MT8173_CLK_MAX];
> + struct clk *clk[CLK_MAX];
should be *[CLK_MAX - 1] but I would prefer to define in the enum:
CLK_MAX = CLK_VENC_LT,
If you are ok with it, I can fix both of my comments when applying.
Regards,
Matthias
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [v17 2/2] drm/bridge: Add I2C based driver for ps8640 bridge
From: Matthias Brugger @ 2016-10-25 12:23 UTC (permalink / raw)
To: Enric Balletbo Serra, Jitao Shi
Cc: David Airlie, Thierry Reding, Mark Rutland, stonea168, dri-devel,
Andy Yan, Ajay Kumar, Vincent Palatin, cawa cheng, bibby.hsieh,
ck.hu, Russell King, devicetree@vger.kernel.org, Sascha Hauer,
Pawel Moll, Ian Campbell, Inki Dae, Rob Herring,
moderated list:ARM/Mediatek SoC support,
Yingjoe Chen <yingjoe>
In-Reply-To: <CAFqH_50_OwsbMWHkT_eukF52BVZYyMbGiamXO=pV4PpJq6BADA@mail.gmail.com>
On 10/18/2016 04:37 PM, Enric Balletbo Serra wrote:
[...]
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
[...]
>> +
>> +/* Firmware */
>> +#define PS_FW_NAME "ps864x_fw.bin"
>> +
>
> From where I can download this firmware image?
>
I suppose this FW bits have to be added to linux-firmware repository
first, before this patch can be accepted.
Regards,
Matthias
^ permalink raw reply
* [PATCH v2] drm/mediatek: fixed the calc method of data rate per lane
From: Jitao Shi @ 2016-10-25 5:40 UTC (permalink / raw)
To: Philipp Zabel, ck.hu-NuS5LvNUpcJWk0Htik3J/w, Matthias Brugger
Cc: Mark Rutland, stonea168-9Onoh4P/yGk,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Andy Yan, Ajay Kumar,
Vincent Palatin, cawa.cheng-NuS5LvNUpcJWk0Htik3J/w,
bibby.hsieh-NuS5LvNUpcJWk0Htik3J/w, Russell King,
devicetree-u79uwXL29TY76Z2rM5mHXA, Jitao Shi, Pawel Moll,
Ian Campbell, Inki Dae, Rob Herring,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w,
eddie.huang-NuS5LvNUpcJWk0Htik3J/w,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rahul Sharma,
srv_heupstream-NuS5LvNUpcJWk0Htik3J/w,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Sascha Hauer, Kumar Gala,
Sean Paul
Tune dsi frame rate by pixel clock, dsi add some extra signal (i.e. Tlpx,
Ths-prepare, Ths-zero, Ths-trail,Ths-exit) when enter and exit LP mode, this
signal will cause h-time larger than normal and reduce FPS.
Need to multiply a coefficient to offset the extra signal's effect.
coefficient = ((htotal*bpp/lane_number)+Tlpx+Ths_prep+Ths_zero+Ths_trail+
Ths_exit)/(htotal*bpp/lane_number))
Signed-off-by: Jitao Shi <jitao.shi-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
Change since v1:
- phy_timing2 and phy_timing3 refer clock cycle time.
- define values of LPX HS_PRPR HS_ZERO HS_TRAIL TA_GO TA_SURE TA_GET DA_HS_EXIT
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 103 +++++++++++++++++++++++-------------
1 file changed, 67 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 28b2044..ade6f46 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -85,16 +85,16 @@
#define LD0_WAKEUP_EN BIT(2)
#define DSI_PHY_TIMECON0 0x110
-#define LPX (0xff << 0)
-#define HS_PRPR (0xff << 8)
-#define HS_ZERO (0xff << 16)
-#define HS_TRAIL (0xff << 24)
+#define LPX (5 << 0)
+#define HS_PRPR (6 << 8)
+#define HS_ZERO (10 << 16)
+#define HS_TRAIL (8 << 24)
#define DSI_PHY_TIMECON1 0x114
-#define TA_GO (0xff << 0)
-#define TA_SURE (0xff << 8)
-#define TA_GET (0xff << 16)
-#define DA_HS_EXIT (0xff << 24)
+#define TA_GO (20 << 0)
+#define TA_SURE (7 << 8)
+#define TA_GET (25 << 16)
+#define DA_HS_EXIT (7 << 24)
#define DSI_PHY_TIMECON2 0x118
#define CONT_DET (0xff << 0)
@@ -158,28 +158,14 @@ static void mtk_dsi_mask(struct mtk_dsi *dsi, u32 offset, u32 mask, u32 data)
writel((temp & ~mask) | (data & mask), dsi->regs + offset);
}
-static void dsi_phy_timconfig(struct mtk_dsi *dsi)
+static void dsi_phy_timconfig(struct mtk_dsi *dsi, u32 phy_timing0,
+ u32 phy_timing1, u32 phy_timing2,
+ u32 phy_timing3)
{
- u32 timcon0, timcon1, timcon2, timcon3;
- unsigned int ui, cycle_time;
- unsigned int lpx;
-
- ui = 1000 / dsi->data_rate + 0x01;
- cycle_time = 8000 / dsi->data_rate + 0x01;
- lpx = 5;
-
- timcon0 = (8 << 24) | (0xa << 16) | (0x6 << 8) | lpx;
- timcon1 = (7 << 24) | (5 * lpx << 16) | ((3 * lpx) / 2) << 8 |
- (4 * lpx);
- timcon2 = ((NS_TO_CYCLE(0x64, cycle_time) + 0xa) << 24) |
- (NS_TO_CYCLE(0x150, cycle_time) << 16);
- timcon3 = (2 * lpx) << 16 | NS_TO_CYCLE(80 + 52 * ui, cycle_time) << 8 |
- NS_TO_CYCLE(0x40, cycle_time);
-
- writel(timcon0, dsi->regs + DSI_PHY_TIMECON0);
- writel(timcon1, dsi->regs + DSI_PHY_TIMECON1);
- writel(timcon2, dsi->regs + DSI_PHY_TIMECON2);
- writel(timcon3, dsi->regs + DSI_PHY_TIMECON3);
+ writel(phy_timing0, dsi->regs + DSI_PHY_TIMECON0);
+ writel(phy_timing1, dsi->regs + DSI_PHY_TIMECON1);
+ writel(phy_timing2, dsi->regs + DSI_PHY_TIMECON2);
+ writel(phy_timing3, dsi->regs + DSI_PHY_TIMECON3);
}
static void mtk_dsi_enable(struct mtk_dsi *dsi)
@@ -202,19 +188,51 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
{
struct device *dev = dsi->dev;
int ret;
+ u64 bit_clock, total_bits;
+ u32 htotal, htotal_bits, bit_per_pixel, overhead_cycles, overhead_bits;
+ u32 phy_timing0, phy_timing1, phy_timing2, phy_timing3;
+ u32 ui, cycle_time;
if (++dsi->refcount != 1)
return 0;
+ switch (dsi->format) {
+ case MIPI_DSI_FMT_RGB565:
+ bit_per_pixel = 16;
+ break;
+ case MIPI_DSI_FMT_RGB666_PACKED:
+ bit_per_pixel = 18;
+ break;
+ case MIPI_DSI_FMT_RGB666:
+ case MIPI_DSI_FMT_RGB888:
+ default:
+ bit_per_pixel = 24;
+ break;
+ }
+ /**
+ * data_rate = (pixel_clock) * bit_per_pixel * mipi_ratio / lane_num;
+ * vm.pixelclock is Khz, data_rata unit is Hz, so need to multiply 1000
+ * mipi_ratio is (htotal * byte_per_pixel / lane_num + Tlpx + Ths_prep
+ * + Thstrail + Ths_exit + Ths_zero) /
+ * (htotal * byte_per_pixel /lane_number)
+ */
+ bit_clock = dsi->vm.pixelclock * 1000 * bit_per_pixel;
+ htotal = dsi->vm.hactive + dsi->vm.hback_porch + dsi->vm.hfront_porch +
+ dsi->vm.hsync_len;
+ htotal_bits = htotal * bit_per_pixel;
+
/**
- * data_rate = (pixel_clock / 1000) * pixel_dipth * mipi_ratio;
- * pixel_clock unit is Khz, data_rata unit is MHz, so need divide 1000.
- * mipi_ratio is mipi clk coefficient for balance the pixel clk in mipi.
- * we set mipi_ratio is 1.05.
+ * overhead = lpx + hs_prepare + hs_zero + hs_trail + hs_exit
*/
- dsi->data_rate = dsi->vm.pixelclock * 3 * 21 / (1 * 1000 * 10);
+ overhead_cycles = LPX + (HS_PRPR >> 8) + (HS_ZERO >> 16) +
+ (HS_TRAIL >> 24) + (DA_HS_EXIT >> 24);
+ overhead_bits = overhead_cycles * dsi->lanes * 8;
+ total_bits = htotal_bits + overhead_bits;
- ret = clk_set_rate(dsi->hs_clk, dsi->data_rate * 1000000);
+ dsi->data_rate = DIV_ROUND_UP_ULL(bit_clock * total_bits,
+ htotal_bits * dsi->lanes);
+
+ ret = clk_set_rate(dsi->hs_clk, dsi->data_rate);
if (ret < 0) {
dev_err(dev, "Failed to set data rate: %d\n", ret);
goto err_refcount;
@@ -236,7 +254,20 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
mtk_dsi_enable(dsi);
mtk_dsi_reset(dsi);
- dsi_phy_timconfig(dsi);
+
+ ui = 1000 / dsi->data_rate + 0x01;
+ cycle_time = 8000 / dsi->data_rate + 0x01;
+
+ phy_timing0 = LPX | HS_PRPR | HS_ZERO | HS_TRAIL;
+ phy_timing1 = TA_GO | TA_SURE | TA_GET | DA_HS_EXIT;
+ phy_timing2 = ((NS_TO_CYCLE(0x64, cycle_time) + 0xa) << 24) |
+ (NS_TO_CYCLE(0x150, cycle_time) << 16);
+ phy_timing3 = (2 * LPX) << 16 |
+ NS_TO_CYCLE(80 + 52 * ui, cycle_time) << 8 |
+ NS_TO_CYCLE(0x40, cycle_time);
+
+ dsi_phy_timconfig(dsi, phy_timing0, phy_timing1, phy_timing2,
+ phy_timing3);
return 0;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] drm: convert DT component matching to component_match_add_release()
From: Jyri Sarha @ 2016-10-24 13:21 UTC (permalink / raw)
To: Russell King, dri-devel, David Airlie
Cc: Liviu Dudau, Chen-Yu Tsai, Xinliang Liu, linux-rockchip,
Tomi Valkeinen, Mali DP Maintainers, linux-arm-msm, Chen Feng,
linux-mediatek, Matthias Brugger, Vincent Abriou,
linux-arm-kernel, Maxime Ripard, freedreno
In-Reply-To: <E1bwo6l-0005Io-Q1@rmk-PC.armlinux.org.uk>
On 10/19/16 13:28, Russell King wrote:
> Convert DT component matching to use component_match_add_release().
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Acked-by: Jyri Sarha <jsarha@ti.com>
Reviewed-by: Jyri Sarha <jsarha@ti.com>
However, the patch description is a bit brief. It took me this mail
thread to fully understand why simple comparison of pointer values may
cause problems if the of_node ref count goes to zero.
BR,
Jyri
> ---
> Can we please get this patch from May merged into the drm-misc or
> whatever trees so that we don't end up with conflicts? I've no idea
> who looks after drm-misc, as they have _still_ failed to add
> themselves to MAINTAINERS.
>
> drivers/gpu/drm/arm/hdlcd_drv.c | 3 ++-
> drivers/gpu/drm/arm/malidp_drv.c | 4 +++-
> drivers/gpu/drm/armada/armada_drv.c | 2 +-
> drivers/gpu/drm/drm_of.c | 28 +++++++++++++++++++++++--
> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 5 +++--
> drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 7 ++++---
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 +++-
> drivers/gpu/drm/msm/msm_drv.c | 12 ++++++-----
> drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 6 ++++--
> drivers/gpu/drm/sti/sti_drv.c | 5 +++--
> drivers/gpu/drm/sun4i/sun4i_drv.c | 3 ++-
> drivers/gpu/drm/tilcdc/tilcdc_external.c | 4 +++-
> include/drm/drm_of.h | 12 +++++++++++
> 13 files changed, 73 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
> index fb6a418ce6be..6477d1a65266 100644
> --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> @@ -453,7 +453,8 @@ static int hdlcd_probe(struct platform_device *pdev)
> return -EAGAIN;
> }
>
> - component_match_add(&pdev->dev, &match, compare_dev, port);
> + drm_of_component_match_add(&pdev->dev, &match, compare_dev, port);
> + of_node_put(port);
>
> return component_master_add_with_match(&pdev->dev, &hdlcd_master_ops,
> match);
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 9280358b8f15..9f4739452a25 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -493,7 +493,9 @@ static int malidp_platform_probe(struct platform_device *pdev)
> return -EAGAIN;
> }
>
> - component_match_add(&pdev->dev, &match, malidp_compare_dev, port);
> + drm_of_component_match_add(&pdev->dev, &match, malidp_compare_dev,
> + port);
> + of_node_put(port);
> return component_master_add_with_match(&pdev->dev, &malidp_master_ops,
> match);
> }
> diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
> index 1e0e68f608e4..94e46da9a758 100644
> --- a/drivers/gpu/drm/armada/armada_drv.c
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -254,7 +254,7 @@ static void armada_add_endpoints(struct device *dev,
> continue;
> }
>
> - component_match_add(dev, match, compare_of, remote);
> + drm_of_component_match_add(dev, match, compare_of, remote);
> of_node_put(remote);
> }
> }
> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> index bc98bb94264d..47848ed8ca48 100644
> --- a/drivers/gpu/drm/drm_of.c
> +++ b/drivers/gpu/drm/drm_of.c
> @@ -6,6 +6,11 @@
> #include <drm/drm_crtc.h>
> #include <drm/drm_of.h>
>
> +static void drm_release_of(struct device *dev, void *data)
> +{
> + of_node_put(data);
> +}
> +
> /**
> * drm_crtc_port_mask - find the mask of a registered CRTC by port OF node
> * @dev: DRM device
> @@ -64,6 +69,24 @@ uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
> EXPORT_SYMBOL(drm_of_find_possible_crtcs);
>
> /**
> + * drm_of_component_match_add - Add a component helper OF node match rule
> + * @master: master device
> + * @matchptr: component match pointer
> + * @compare: compare function used for matching component
> + * @node: of_node
> + */
> +void drm_of_component_match_add(struct device *master,
> + struct component_match **matchptr,
> + int (*compare)(struct device *, void *),
> + struct device_node *node)
> +{
> + of_node_get(node);
> + component_match_add_release(master, matchptr, drm_release_of,
> + compare, node);
> +}
> +EXPORT_SYMBOL_GPL(drm_of_component_match_add);
> +
> +/**
> * drm_of_component_probe - Generic probe function for a component based master
> * @dev: master device containing the OF node
> * @compare_of: compare function used for matching components
> @@ -101,7 +124,7 @@ int drm_of_component_probe(struct device *dev,
> continue;
> }
>
> - component_match_add(dev, &match, compare_of, port);
> + drm_of_component_match_add(dev, &match, compare_of, port);
> of_node_put(port);
> }
>
> @@ -140,7 +163,8 @@ int drm_of_component_probe(struct device *dev,
> continue;
> }
>
> - component_match_add(dev, &match, compare_of, remote);
> + drm_of_component_match_add(dev, &match, compare_of,
> + remote);
> of_node_put(remote);
> }
> of_node_put(port);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index aa687669e22b..0dee6acbd880 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -16,6 +16,7 @@
>
> #include <linux/component.h>
> #include <linux/of_platform.h>
> +#include <drm/drm_of.h>
>
> #include "etnaviv_drv.h"
> #include "etnaviv_gpu.h"
> @@ -629,8 +630,8 @@ static int etnaviv_pdev_probe(struct platform_device *pdev)
> if (!core_node)
> break;
>
> - component_match_add(&pdev->dev, &match, compare_of,
> - core_node);
> + drm_of_component_match_add(&pdev->dev, &match,
> + compare_of, core_node);
> of_node_put(core_node);
> }
> } else if (dev->platform_data) {
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> index 90377a609c98..e88fde18c946 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> @@ -24,6 +24,7 @@
> #include <drm/drm_fb_cma_helper.h>
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_crtc_helper.h>
> +#include <drm/drm_of.h>
>
> #include "kirin_drm_drv.h"
>
> @@ -260,14 +261,13 @@ static struct device_node *kirin_get_remote_node(struct device_node *np)
> DRM_ERROR("no valid endpoint node\n");
> return ERR_PTR(-ENODEV);
> }
> - of_node_put(endpoint);
>
> remote = of_graph_get_remote_port_parent(endpoint);
> + of_node_put(endpoint);
> if (!remote) {
> DRM_ERROR("no valid remote node\n");
> return ERR_PTR(-ENODEV);
> }
> - of_node_put(remote);
>
> if (!of_device_is_available(remote)) {
> DRM_ERROR("not available for remote node\n");
> @@ -294,7 +294,8 @@ static int kirin_drm_platform_probe(struct platform_device *pdev)
> if (IS_ERR(remote))
> return PTR_ERR(remote);
>
> - component_match_add(dev, &match, compare_of, remote);
> + drm_of_component_match_add(dev, &match, compare_of, remote);
> + of_node_put(remote);
>
> return component_master_add_with_match(dev, &kirin_drm_ops, match);
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index cf83f6507ec8..9c5430fb82a2 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -18,6 +18,7 @@
> #include <drm/drm_crtc_helper.h>
> #include <drm/drm_gem.h>
> #include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_of.h>
> #include <linux/component.h>
> #include <linux/iommu.h>
> #include <linux/of_address.h>
> @@ -415,7 +416,8 @@ static int mtk_drm_probe(struct platform_device *pdev)
> comp_type == MTK_DPI) {
> dev_info(dev, "Adding component match for %s\n",
> node->full_name);
> - component_match_add(dev, &match, compare_of, node);
> + drm_of_component_match_add(dev, &match, compare_of,
> + node);
> } else {
> struct mtk_ddp_comp *comp;
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index fb5c0b0a7594..84d38eaea585 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -15,6 +15,8 @@
> * this program. If not, see <http://www.gnu.org/licenses/>.
> */
>
> +#include <drm/drm_of.h>
> +
> #include "msm_drv.h"
> #include "msm_debugfs.h"
> #include "msm_fence.h"
> @@ -919,8 +921,8 @@ static int add_components_mdp(struct device *mdp_dev,
> continue;
> }
>
> - component_match_add(master_dev, matchptr, compare_of, intf);
> -
> + drm_of_component_match_add(master_dev, matchptr, compare_of,
> + intf);
> of_node_put(intf);
> of_node_put(ep_node);
> }
> @@ -962,8 +964,8 @@ static int add_display_components(struct device *dev,
> put_device(mdp_dev);
>
> /* add the MDP component itself */
> - component_match_add(dev, matchptr, compare_of,
> - mdp_dev->of_node);
> + drm_of_component_match_add(dev, matchptr, compare_of,
> + mdp_dev->of_node);
> } else {
> /* MDP4 */
> mdp_dev = dev;
> @@ -996,7 +998,7 @@ static int add_gpu_components(struct device *dev,
> if (!np)
> return 0;
>
> - component_match_add(dev, matchptr, compare_of, np);
> + drm_of_component_match_add(dev, matchptr, compare_of, np);
>
> of_node_put(np);
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index 8c8cbe837e61..6fe161192bb4 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -20,6 +20,7 @@
> #include <drm/drm_crtc_helper.h>
> #include <drm/drm_fb_helper.h>
> #include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_of.h>
> #include <linux/dma-mapping.h>
> #include <linux/pm_runtime.h>
> #include <linux/module.h>
> @@ -388,7 +389,7 @@ static void rockchip_add_endpoints(struct device *dev,
> continue;
> }
>
> - component_match_add(dev, match, compare_of, remote);
> + drm_of_component_match_add(dev, match, compare_of, remote);
> of_node_put(remote);
> }
> }
> @@ -437,7 +438,8 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
> }
>
> of_node_put(iommu);
> - component_match_add(dev, &match, compare_of, port->parent);
> + drm_of_component_match_add(dev, &match, compare_of,
> + port->parent);
> of_node_put(port);
> }
>
> diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
> index 2784919a7366..5e819876e642 100644
> --- a/drivers/gpu/drm/sti/sti_drv.c
> +++ b/drivers/gpu/drm/sti/sti_drv.c
> @@ -17,6 +17,7 @@
> #include <drm/drm_crtc_helper.h>
> #include <drm/drm_gem_cma_helper.h>
> #include <drm/drm_fb_cma_helper.h>
> +#include <drm/drm_of.h>
>
> #include "sti_crtc.h"
> #include "sti_drv.h"
> @@ -423,8 +424,8 @@ static int sti_platform_probe(struct platform_device *pdev)
> child_np = of_get_next_available_child(node, NULL);
>
> while (child_np) {
> - component_match_add(dev, &match, compare_of, child_np);
> - of_node_put(child_np);
> + drm_of_component_match_add(dev, &match, compare_of,
> + child_np);
> child_np = of_get_next_available_child(node, child_np);
> }
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
> index 0da9862ad8ed..b3c4ad605e81 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> @@ -18,6 +18,7 @@
> #include <drm/drm_fb_cma_helper.h>
> #include <drm/drm_gem_cma_helper.h>
> #include <drm/drm_fb_helper.h>
> +#include <drm/drm_of.h>
>
> #include "sun4i_crtc.h"
> #include "sun4i_drv.h"
> @@ -239,7 +240,7 @@ static int sun4i_drv_add_endpoints(struct device *dev,
> /* Add current component */
> DRM_DEBUG_DRIVER("Adding component %s\n",
> of_node_full_name(node));
> - component_match_add(dev, match, compare_of, node);
> + drm_of_component_match_add(dev, match, compare_of, node);
> count++;
> }
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> index 68e895021005..06a4c584f3cb 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_external.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> @@ -10,6 +10,7 @@
>
> #include <linux/component.h>
> #include <linux/of_graph.h>
> +#include <drm/drm_of.h>
>
> #include "tilcdc_drv.h"
> #include "tilcdc_external.h"
> @@ -160,7 +161,8 @@ int tilcdc_get_external_components(struct device *dev,
>
> dev_dbg(dev, "Subdevice node '%s' found\n", node->name);
> if (match)
> - component_match_add(dev, match, dev_match_of, node);
> + drm_of_component_match_add(dev, match, dev_match_of,
> + node);
> of_node_put(node);
> count++;
> }
> diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h
> index 3fd87b386ed7..d6b4c5587bbe 100644
> --- a/include/drm/drm_of.h
> +++ b/include/drm/drm_of.h
> @@ -4,6 +4,7 @@
> #include <linux/of_graph.h>
>
> struct component_master_ops;
> +struct component_match;
> struct device;
> struct drm_device;
> struct drm_encoder;
> @@ -12,6 +13,10 @@ struct device_node;
> #ifdef CONFIG_OF
> extern uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
> struct device_node *port);
> +extern void drm_of_component_match_add(struct device *master,
> + struct component_match **matchptr,
> + int (*compare)(struct device *, void *),
> + struct device_node *node);
> extern int drm_of_component_probe(struct device *dev,
> int (*compare_of)(struct device *, void *),
> const struct component_master_ops *m_ops);
> @@ -25,6 +30,13 @@ static inline uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
> return 0;
> }
>
> +static void drm_of_component_match_add(struct device *master,
> + struct component_match **matchptr,
> + int (*compare)(struct device *, void *),
> + struct device_node *node)
> +{
> +}
> +
> static inline int
> drm_of_component_probe(struct device *dev,
> int (*compare_of)(struct device *, void *),
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH 2/4] mtk_mdp_vpu: remove a double unlock at the error path
From: Minghsiu Tsai @ 2016-10-24 11:59 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
Mauro Carvalho Chehab, Matthias Brugger, Hans Verkuil,
linux-arm-kernel, linux-mediatek
In-Reply-To: <768767961c64dea3fdd86132c9eba87ae652d588.1477058332.git.mchehab@s-opensource.com>
On Fri, 2016-10-21 at 11:59 -0200, Mauro Carvalho Chehab wrote:
> As warned by smatch:
> drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c:98 mtk_mdp_vpu_send_msg() error: double unlock 'mutex:&ctx->mdp_dev->vpulock'
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
> drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c b/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c
> index b38d29e99f7a..5c8caa864e32 100644
> --- a/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c
> +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c
> @@ -91,7 +91,6 @@ static int mtk_mdp_vpu_send_msg(void *msg, int len, struct mtk_mdp_vpu *vpu,
> mutex_lock(&ctx->mdp_dev->vpulock);
> err = vpu_ipi_send(vpu->pdev, (enum ipi_id)id, msg, len);
> if (err) {
> - mutex_unlock(&ctx->mdp_dev->vpulock);
Hi Mauro,
It has been fixed by Hans in the later patch.
Author: Hans Verkuil <hverkuil@xs4all.nl>
Date: Mon Sep 19 05:00:34 2016 -0300
[media] mtk-mdp: fix double mutex_unlock
Fix smatch error:
media-git/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c:100
mtk_mdp_vpu_send_msg() error: double unlock 'mutex:&ctx->
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
minghsiu
> dev_err(&ctx->mdp_dev->pdev->dev,
> "vpu_ipi_send fail status %d\n", err);
> }
^ permalink raw reply
* Re: [PATCH 3/4] mtk_mdp_m2m: remove an unused struct
From: Minghsiu Tsai @ 2016-10-24 11:49 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
Mauro Carvalho Chehab, Matthias Brugger, Hans Verkuil,
linux-arm-kernel, linux-mediatek
In-Reply-To: <624b0ea4550b90318ec2293d80b1caa5bafd2a35.1477058332.git.mchehab@s-opensource.com>
On Fri, 2016-10-21 at 11:59 -0200, Mauro Carvalho Chehab wrote:
> drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c:48:33: warning: ‘mtk_mdp_size_align’ defined but not used [-Wunused-variable]
> static struct mtk_mdp_pix_align mtk_mdp_size_align = {
> ^~~~~~~~~~~~~~~~~~
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
> drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c | 7 -------
> 1 file changed, 7 deletions(-)
>
> diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> index 065502757133..33124a6c9951 100644
> --- a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> @@ -45,13 +45,6 @@ struct mtk_mdp_pix_limit {
> u16 target_rot_en_h;
> };
>
> -static struct mtk_mdp_pix_align mtk_mdp_size_align = {
> - .org_w = 16,
> - .org_h = 16,
> - .target_w = 2,
> - .target_h = 2,
> -};
> -
Hi Mauro,
The structure is used for the format V4L2_PIX_FMT_MT21C which is added
in the later patch.
"[media] media: mtk-mdp: support pixelformat V4L2_PIX_FMT_MT21C"
I just know checkpatch should be run patch by patch, so this warning
message will be generated without the MT21C patch.
I found all mtk-mdp patches have been merged in media tree, so is this
patch still needed?
If yes, remove 'mtk_mdp_size_align' in this patch, and re-added it in
the MT21C patch.
minghsiu
> static const struct mtk_mdp_fmt mtk_mdp_formats[] = {
> {
> .pixelformat = V4L2_PIX_FMT_NV12M,
^ permalink raw reply
* Re: [SPAM][PATCH 3/4] mtk_mdp_m2m: remove an unused struct
From: Minghsiu Tsai @ 2016-10-24 11:47 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Mauro Carvalho Chehab, Hans Verkuil, Matthias Brugger,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Mauro Carvalho Chehab,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Linux Media Mailing List
In-Reply-To: <624b0ea4550b90318ec2293d80b1caa5bafd2a35.1477058332.git.mchehab-JsYNTwtnfakRB7SZvlqPiA@public.gmane.org>
On Fri, 2016-10-21 at 11:59 -0200, Mauro Carvalho Chehab wrote:
> drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c:48:33: warning: ‘mtk_mdp_size_align’ defined but not used [-Wunused-variable]
> static struct mtk_mdp_pix_align mtk_mdp_size_align = {
> ^~~~~~~~~~~~~~~~~~
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
> drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c | 7 -------
> 1 file changed, 7 deletions(-)
>
> diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> index 065502757133..33124a6c9951 100644
> --- a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> @@ -45,13 +45,6 @@ struct mtk_mdp_pix_limit {
> u16 target_rot_en_h;
> };
>
> -static struct mtk_mdp_pix_align mtk_mdp_size_align = {
> - .org_w = 16,
> - .org_h = 16,
> - .target_w = 2,
> - .target_h = 2,
> -};
> -
Hi Mauro,
The structure is used for the format V4L2_PIX_FMT_MT21C which is added
in the later patch.
"[media] media: mtk-mdp: support pixelformat V4L2_PIX_FMT_MT21C"
I just know checkpatch should be run patch by patch, so this warning
message will be generated without the MT21C patch.
I found all mtk-mdp patches have been merged in media tree, so is this
patch still needed?
If yes, remove 'mtk_mdp_size_align' in this patch, and re-added it in
the MT21C patch.
minghsiu
> static const struct mtk_mdp_fmt mtk_mdp_formats[] = {
> {
> .pixelformat = V4L2_PIX_FMT_NV12M,
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH 4/4] mtk-mdp: fix compilation warnings if !DEBUG
From: Minghsiu Tsai @ 2016-10-24 11:27 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
Mauro Carvalho Chehab, Matthias Brugger, Hans Verkuil,
linux-arm-kernel, linux-mediatek
In-Reply-To: <5a24855fda4d91a58f119c2d70177017e06fc6f0.1477058332.git.mchehab@s-opensource.com>
On Fri, 2016-10-21 at 11:59 -0200, Mauro Carvalho Chehab wrote:
> The mtk_mdp_dbg() is empty if !DEBUG. This causes the following
> warnings:
>
> drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c: In function ‘mtk_mdp_try_fmt_mplane’:
> drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c:231:52: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
> org_w, org_h, pix_mp->width, pix_mp->height);
> ^
> drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c: In function ‘mtk_mdp_m2m_start_streaming’:
> drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c:414:21: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
> ctx->id, ret);
> ^
>
> With could actually make the code to do something wrong. So,
> add an empty block to make it be parsed ok.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
> drivers/media/platform/mtk-mdp/mtk_mdp_core.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
> index 2e979f97d1df..848569d4ab90 100644
> --- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
> +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
> @@ -250,7 +250,7 @@ extern int mtk_mdp_dbg_level;
>
> #else
>
> -#define mtk_mdp_dbg(level, fmt, args...)
> +#define mtk_mdp_dbg(level, fmt, args...) {}
>
Acked-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
> #define mtk_mdp_err(fmt, args...)
> #define mtk_mdp_dbg_enter()
> #define mtk_mdp_dbg_leave()
^ permalink raw reply
* Re: [PATCH 1/4] mtk_mdp_vpu: fix build with COMPILE_TEST for 32 bits
From: Minghsiu Tsai @ 2016-10-24 11:26 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
Mauro Carvalho Chehab, Matthias Brugger, Hans Verkuil,
linux-arm-kernel, linux-mediatek
In-Reply-To: <cd14afdb178cf490e257368bc899c7a0c690d140.1477058332.git.mchehab@s-opensource.com>
Hi Mauro,
This issue has been fixed by the patch below and merged in media tree,
and also signed by you.
Is it duplicate?
commit 37bf7e34ecc817ce6b8278588aeb22aab5635e1c
Author: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
Date: Mon Sep 19 03:34:42 2016 -0300
[media] media: mtk-mdp: fix build warning in arch x86
This patch fix build warning in arch x86
Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
On Fri, 2016-10-21 at 11:59 -0200, Mauro Carvalho Chehab wrote:
> When building on i386 in 32 bits, several new warnings appear:
>
> drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c: In function 'mtk_mdp_vpu_handle_init_ack':
> drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c:28:28: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
> struct mtk_mdp_vpu *vpu = (struct mtk_mdp_vpu *)msg->ap_inst;
> ^
> drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c: In function 'mtk_mdp_vpu_ipi_handler':
> drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c:40:28: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
> struct mtk_mdp_vpu *vpu = (struct mtk_mdp_vpu *)msg->ap_inst;
> ^
> drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c: In function 'mtk_mdp_vpu_send_ap_ipi':
> drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c:111:16: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
> msg.ap_inst = (uint64_t)vpu;
> ^
> drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c: In function 'mtk_mdp_vpu_init':
> drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c:129:16: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
> msg.ap_inst = (uint64_t)vpu;
> ^
>
> That's because the driver assumes that it will be built only on
> 64 bits. As we don't want extra warnings when building with 32
> bits, we need to double-cast.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
> drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c b/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c
> index fb07bf3dbd8b..b38d29e99f7a 100644
> --- a/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c
> +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c
> @@ -25,7 +25,7 @@ static inline struct mtk_mdp_ctx *vpu_to_ctx(struct mtk_mdp_vpu *vpu)
>
> static void mtk_mdp_vpu_handle_init_ack(struct mdp_ipi_comm_ack *msg)
> {
> - struct mtk_mdp_vpu *vpu = (struct mtk_mdp_vpu *)msg->ap_inst;
> + struct mtk_mdp_vpu *vpu = (struct mtk_mdp_vpu *)(long)msg->ap_inst;
>
> /* mapping VPU address to kernel virtual address */
> vpu->vsi = (struct mdp_process_vsi *)
> @@ -37,7 +37,7 @@ static void mtk_mdp_vpu_ipi_handler(void *data, unsigned int len, void *priv)
> {
> unsigned int msg_id = *(unsigned int *)data;
> struct mdp_ipi_comm_ack *msg = (struct mdp_ipi_comm_ack *)data;
> - struct mtk_mdp_vpu *vpu = (struct mtk_mdp_vpu *)msg->ap_inst;
> + struct mtk_mdp_vpu *vpu = (struct mtk_mdp_vpu *)(long)msg->ap_inst;
> struct mtk_mdp_ctx *ctx;
>
> vpu->failure = msg->status;
> @@ -108,7 +108,7 @@ static int mtk_mdp_vpu_send_ap_ipi(struct mtk_mdp_vpu *vpu, uint32_t msg_id)
> msg.msg_id = msg_id;
> msg.ipi_id = IPI_MDP;
> msg.vpu_inst_addr = vpu->inst_addr;
> - msg.ap_inst = (uint64_t)vpu;
> + msg.ap_inst = (uint64_t)(long)vpu;
> err = mtk_mdp_vpu_send_msg((void *)&msg, sizeof(msg), vpu, IPI_MDP);
> if (!err && vpu->failure)
> err = -EINVAL;
> @@ -126,7 +126,7 @@ int mtk_mdp_vpu_init(struct mtk_mdp_vpu *vpu)
>
> msg.msg_id = AP_MDP_INIT;
> msg.ipi_id = IPI_MDP;
> - msg.ap_inst = (uint64_t)vpu;
> + msg.ap_inst = (uint64_t)(long)vpu;
> err = mtk_mdp_vpu_send_msg((void *)&msg, sizeof(msg), vpu, IPI_MDP);
> if (!err && vpu->failure)
> err = -EINVAL;
^ permalink raw reply
* Re: [PATCH v5 3/9] vcodec: mediatek: Add Mediatek V4L2 Video Decoder Driver
From: Tiffany Lin @ 2016-10-24 10:49 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Hans Verkuil, daniel.thompson-QSEj5FYQhm4dnm+yROfE0A, Rob Herring,
Matthias Brugger, Daniel Kurtz, Pawel Osciak, Eddie Huang,
Yingjoe Chen, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
PoChun.Lin-NuS5LvNUpcJWk0Htik3J/w
In-Reply-To: <20161024070542.65e11e37-ch4gOOMV7nf/PtFMR13I2A@public.gmane.org>
Hi Mauro,
On Mon, 2016-10-24 at 07:05 -0200, Mauro Carvalho Chehab wrote:
> Em Mon, 24 Oct 2016 11:22:08 +0800
> Tiffany Lin <tiffany.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> escreveu:
>
> > Hi Mauro,
> >
> > On Fri, 2016-10-21 at 11:01 -0200, Mauro Carvalho Chehab wrote:
> > > Em Fri, 2 Sep 2016 20:19:54 +0800
> > > Tiffany Lin <tiffany.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> escreveu:
> > >
> > > > Add v4l2 layer decoder driver for MT8173
> > > >
> > > > Signed-off-by: Tiffany Lin <tiffany.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > >
> > > > +int vdec_if_init(struct mtk_vcodec_ctx *ctx, unsigned int fourcc)
> > > > +{
> > > > + int ret = 0;
> > > > +
> > > > + switch (fourcc) {
> > > > + case V4L2_PIX_FMT_H264:
> > > > + case V4L2_PIX_FMT_VP8:
> > > > + default:
> > > > + return -EINVAL;
> > > > + }
> > >
> > > Did you ever test this driver? The above code will *always* return
> > > -EINVAL, with will cause vidioc_vdec_s_fmt() to always fail!
> > >
> > > I suspect that what you wanted to do, instead, is:
> > >
> > > switch (fourcc) {
> > > case V4L2_PIX_FMT_H264:
> > > case V4L2_PIX_FMT_VP8:
> > > break;
> > > default:
> > > return -EINVAL;
> > >
> >
> > The original idea here is that vp8 and h264 are added in later patches.
> > If get this patch without later patches, it should return -EINVAL.
>
> I noticed your idea, but next time, don't add dead code like that.
> Reviewers check patch by patch at the order they're present at the
> patch series.
>
> So, don't add something broken by purpose, assuming that it would
> be fixed later.
>
Got it.
> >
> >
> > > Btw, this patch series has also several issues that were pointed by
> > > checkpatch. Please *always* run checkpatch when submitting your work.
> > >
> > > You should take a look at the Kernel documentation about how to
> > > submit patches, at:
> > > https://mchehab.fedorapeople.org/kernel_docs/process/index.html
> > >
> > > PS.: this time, I fixed the checkpatch issues for you. So, let me know
> > > if the patch below is OK, and I'll merge it at media upstream,
> > > assuming that the other patches in this series are ok.
> > >
> >
> > I did run checkpatch, but I don't know why these issues missed.
> > probably I run checkpatch for all files not for patches.
> > I will take a look at the documentation and keep this in mind for future
> > upstream.
> > Appreciated for your help.
>
> Checkpatch should be run patch by patch, as we expect that all patches
> will follow the coding style and will compile fine, without introducing
> warnings.
>
> I do compile the Kernel for every single patch I merge.
>
Got it. I will follow this.
best regards,
Tiffany
> Regards,
> Mauro
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 3/9] vcodec: mediatek: Add Mediatek V4L2 Video Decoder Driver
From: Mauro Carvalho Chehab @ 2016-10-24 9:05 UTC (permalink / raw)
To: Tiffany Lin
Cc: Hans Verkuil, daniel.thompson, Rob Herring, Matthias Brugger,
Daniel Kurtz, Pawel Osciak, Eddie Huang, Yingjoe Chen, devicetree,
linux-kernel, linux-arm-kernel, linux-media, linux-mediatek,
PoChun.Lin
In-Reply-To: <1477279328.10501.10.camel@mtksdaap41>
Em Mon, 24 Oct 2016 11:22:08 +0800
Tiffany Lin <tiffany.lin@mediatek.com> escreveu:
> Hi Mauro,
>
> On Fri, 2016-10-21 at 11:01 -0200, Mauro Carvalho Chehab wrote:
> > Em Fri, 2 Sep 2016 20:19:54 +0800
> > Tiffany Lin <tiffany.lin@mediatek.com> escreveu:
> >
> > > Add v4l2 layer decoder driver for MT8173
> > >
> > > Signed-off-by: Tiffany Lin <tiffany.lin@mediatek.com>
> >
> > > +int vdec_if_init(struct mtk_vcodec_ctx *ctx, unsigned int fourcc)
> > > +{
> > > + int ret = 0;
> > > +
> > > + switch (fourcc) {
> > > + case V4L2_PIX_FMT_H264:
> > > + case V4L2_PIX_FMT_VP8:
> > > + default:
> > > + return -EINVAL;
> > > + }
> >
> > Did you ever test this driver? The above code will *always* return
> > -EINVAL, with will cause vidioc_vdec_s_fmt() to always fail!
> >
> > I suspect that what you wanted to do, instead, is:
> >
> > switch (fourcc) {
> > case V4L2_PIX_FMT_H264:
> > case V4L2_PIX_FMT_VP8:
> > break;
> > default:
> > return -EINVAL;
> >
>
> The original idea here is that vp8 and h264 are added in later patches.
> If get this patch without later patches, it should return -EINVAL.
I noticed your idea, but next time, don't add dead code like that.
Reviewers check patch by patch at the order they're present at the
patch series.
So, don't add something broken by purpose, assuming that it would
be fixed later.
>
>
> > Btw, this patch series has also several issues that were pointed by
> > checkpatch. Please *always* run checkpatch when submitting your work.
> >
> > You should take a look at the Kernel documentation about how to
> > submit patches, at:
> > https://mchehab.fedorapeople.org/kernel_docs/process/index.html
> >
> > PS.: this time, I fixed the checkpatch issues for you. So, let me know
> > if the patch below is OK, and I'll merge it at media upstream,
> > assuming that the other patches in this series are ok.
> >
>
> I did run checkpatch, but I don't know why these issues missed.
> probably I run checkpatch for all files not for patches.
> I will take a look at the documentation and keep this in mind for future
> upstream.
> Appreciated for your help.
Checkpatch should be run patch by patch, as we expect that all patches
will follow the coding style and will compile fine, without introducing
warnings.
I do compile the Kernel for every single patch I merge.
Regards,
Mauro
^ permalink raw reply
* Re: FW: [PATCH] mtk-vcodec: fix some smatch warnings
From: Tiffany Lin @ 2016-10-24 3:49 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: daniel.thompson-QSEj5FYQhm4dnm+yROfE0A, Hans Verkuil, Rob Herring,
Matthias Brugger, Daniel Kurtz, Pawel Osciak, Eddie Huang,
Yingjoe Chen, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
PoChun.Lin-NuS5LvNUpcJWk0Htik3J/w
In-Reply-To: <6D1A1B40F9E1B64689A7C1952F8B336792FED2F0@mtkmbs08n1>
> Fix this bug:
> drivers/media/platform/mtk-vcodec/vdec_drv_if.c:38 vdec_if_init() info: ignoring unreachable code.
>
> With is indeed a real problem that prevents the driver to work!
>
> While here, also remove an used var, as reported by smatch:
>
> drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c: In function 'mtk_vcodec_init_dec_pm':
> drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c:29:17: warning: variable 'dev' set but not used [-Wunused-but-set-variable]
> struct device *dev;
> ^~~
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab-JsYNTwtnfakRB7SZvlqPiA@public.gmane.org>
Acked-by: Tiffany Lin <tiffany.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
> drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c | 2 --
> drivers/media/platform/mtk-vcodec/vdec_drv_if.c | 1 +
> 2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c
> index 18182f5676d8..79ca03ac449c 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c
> @@ -26,14 +26,12 @@ int mtk_vcodec_init_dec_pm(struct mtk_vcodec_dev *mtkdev) {
> struct device_node *node;
> struct platform_device *pdev;
> - struct device *dev;
> struct mtk_vcodec_pm *pm;
> int ret = 0;
>
> pdev = mtkdev->plat_dev;
> pm = &mtkdev->pm;
> pm->mtkdev = mtkdev;
> - dev = &pdev->dev;
> node = of_parse_phandle(pdev->dev.of_node, "mediatek,larb", 0);
> if (!node) {
> mtk_v4l2_err("of_parse_phandle mediatek,larb fail!"); diff --git a/drivers/media/platform/mtk-vcodec/vdec_drv_if.c b/drivers/media/platform/mtk-vcodec/vdec_drv_if.c
> index 3cb04ef45144..9813b2ffd5fa 100644
> --- a/drivers/media/platform/mtk-vcodec/vdec_drv_if.c
> +++ b/drivers/media/platform/mtk-vcodec/vdec_drv_if.c
> @@ -31,6 +31,7 @@ int vdec_if_init(struct mtk_vcodec_ctx *ctx, unsigned int fourcc)
> switch (fourcc) {
> case V4L2_PIX_FMT_H264:
> case V4L2_PIX_FMT_VP8:
> + break;
> default:
> return -EINVAL;
> }
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 3/9] vcodec: mediatek: Add Mediatek V4L2 Video Decoder Driver
From: Tiffany Lin @ 2016-10-24 3:22 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Hans Verkuil, daniel.thompson, Rob Herring, Matthias Brugger,
Daniel Kurtz, Pawel Osciak, Eddie Huang, Yingjoe Chen, devicetree,
linux-kernel, linux-arm-kernel, linux-media, linux-mediatek,
PoChun.Lin
In-Reply-To: <20161021110104.5733240e@vento.lan>
Hi Mauro,
On Fri, 2016-10-21 at 11:01 -0200, Mauro Carvalho Chehab wrote:
> Em Fri, 2 Sep 2016 20:19:54 +0800
> Tiffany Lin <tiffany.lin@mediatek.com> escreveu:
>
> > Add v4l2 layer decoder driver for MT8173
> >
> > Signed-off-by: Tiffany Lin <tiffany.lin@mediatek.com>
>
> > +int vdec_if_init(struct mtk_vcodec_ctx *ctx, unsigned int fourcc)
> > +{
> > + int ret = 0;
> > +
> > + switch (fourcc) {
> > + case V4L2_PIX_FMT_H264:
> > + case V4L2_PIX_FMT_VP8:
> > + default:
> > + return -EINVAL;
> > + }
>
> Did you ever test this driver? The above code will *always* return
> -EINVAL, with will cause vidioc_vdec_s_fmt() to always fail!
>
> I suspect that what you wanted to do, instead, is:
>
> switch (fourcc) {
> case V4L2_PIX_FMT_H264:
> case V4L2_PIX_FMT_VP8:
> break;
> default:
> return -EINVAL;
>
The original idea here is that vp8 and h264 are added in later patches.
If get this patch without later patches, it should return -EINVAL.
> Btw, this patch series has also several issues that were pointed by
> checkpatch. Please *always* run checkpatch when submitting your work.
>
> You should take a look at the Kernel documentation about how to
> submit patches, at:
> https://mchehab.fedorapeople.org/kernel_docs/process/index.html
>
> PS.: this time, I fixed the checkpatch issues for you. So, let me know
> if the patch below is OK, and I'll merge it at media upstream,
> assuming that the other patches in this series are ok.
>
I did run checkpatch, but I don't know why these issues missed.
probably I run checkpatch for all files not for patches.
I will take a look at the documentation and keep this in mind for future
upstream.
Appreciated for your help.
best regards,
Tiffany
^ permalink raw reply
* RE: [PATCH -next] ARM: mediatek: add terminate entry for of_device_id tables
From: weiyongjun (A) @ 2016-10-24 0:24 UTC (permalink / raw)
To: Wei Yongjun, Matthias Brugger, Russell King
Cc: linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <1477222977-1336-1-git-send-email-weiyj.lk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> -----Original Message-----
> From: Wei Yongjun [mailto:weiyj.lk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> Sent: Sunday, October 23, 2016 7:43 PM
> To: Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; Russell King
> <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
> Cc: weiyongjun (A) <weiyongjun1-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>; linux-arm-
> kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; linux-
> kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: [PATCH -next] ARM: mediatek: add terminate entry for
> of_device_id tables
>
> From: Wei Yongjun <weiyongjun1-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>
> Make sure of_device_id tables are NULL terminated.
>
> Signed-off-by: Wei Yongjun <weiyongjun1-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
> arch/arm/mach-mediatek/platsmp.c | 2 ++
> 1 file changed, 2 insertions(+)
>
Sorry, this patch is not correct since mtk_tz_smp_boot_infos is used
as ARRAY_SIZE(mtk_tz_smp_boot_infos), please ignore it.
Regards,
Yongjun Wei
^ permalink raw reply
* [PATCH -next] ARM: mediatek: add terminate entry for of_device_id tables
From: Wei Yongjun @ 2016-10-23 11:42 UTC (permalink / raw)
To: Matthias Brugger, Russell King
Cc: Wei Yongjun, linux-arm-kernel, linux-mediatek, linux-kernel
From: Wei Yongjun <weiyongjun1@huawei.com>
Make sure of_device_id tables are NULL terminated.
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
arch/arm/mach-mediatek/platsmp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/mach-mediatek/platsmp.c b/arch/arm/mach-mediatek/platsmp.c
index b821e34..e6cffc7 100644
--- a/arch/arm/mach-mediatek/platsmp.c
+++ b/arch/arm/mach-mediatek/platsmp.c
@@ -54,11 +54,13 @@ static const struct of_device_id mtk_tz_smp_boot_infos[] __initconst = {
{ .compatible = "mediatek,mt8135", .data = &mtk_mt8135_tz_boot },
{ .compatible = "mediatek,mt8127", .data = &mtk_mt8135_tz_boot },
{ .compatible = "mediatek,mt2701", .data = &mtk_mt8135_tz_boot },
+ { },
};
static const struct of_device_id mtk_smp_boot_infos[] __initconst = {
{ .compatible = "mediatek,mt6589", .data = &mtk_mt6589_boot },
{ .compatible = "mediatek,mt7623", .data = &mtk_mt7623_boot },
+ { },
};
static void __iomem *mtk_smp_base;
^ permalink raw reply related
* Re: [PATCH] drm: convert DT component matching to component_match_add_release()
From: Russell King - ARM Linux @ 2016-10-21 15:23 UTC (permalink / raw)
To: Sean Paul
Cc: Heiko Stuebner, David Airlie, Liviu Dudau, dri-devel,
Benjamin Gaignard, Chen-Yu Tsai, Xinliang Liu,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Xinwei Kong,
Tomi Valkeinen, Mali DP Maintainers, linux-arm-msm, CK Hu,
Chen Feng, Jyri Sarha, Christian Gmeiner,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Matthias Brugger,
Vincent Abriou, Linux ARM Kernel, Mark Yao, Rob Clark, Philipp
In-Reply-To: <CAOw6vbJ64JOugR2b3R87pFRDGDVs9HM3uZJM2z1-jHyhPf6WOA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Thu, Oct 20, 2016 at 07:15:56PM -0400, Sean Paul wrote:
> On Thu, Oct 20, 2016 at 5:50 PM, Russell King - ARM Linux
> <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:
> > On Thu, Oct 20, 2016 at 04:39:04PM -0400, Sean Paul wrote:
> >> On Wed, Oct 19, 2016 at 6:28 AM, Russell King
> >> <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:
> >> > Convert DT component matching to use component_match_add_release().
> >> >
> >> > Signed-off-by: Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
> >> > ---
> >> > Can we please get this patch from May merged into the drm-misc or
> >> > whatever trees so that we don't end up with conflicts? I've no idea
> >> > who looks after drm-misc, as they have _still_ failed to add
> >> > themselves to MAINTAINERS.
> >>
> >> I think Daniel explained pretty clearly why this wasn't happening in
> >> the previous thread.
> >>
> >> Next time you send a v2, can you please mark it as such and include a
> >> "Changes in v2" section?
> >
> > Why - nothing's changed other than a rebase onto 4.9-rc1. This isn't
> > a "I've changed it in XYZ way, so here's a new copy".
>
>
> Changes in v2: None
>
> Is still useful since:
> a) the diffstat is different from v1, which necessitates me going
> through both versions to see what's changed from the previous reviews
> (only to find out myself that it's been rebased and a function has
> changed names)
Sorry, but I don't track in any fine detail what the changes are
between patches when I bring them forward: I have far too many patches
to do that (I have 300 right now) and I never know whether they're
going to result in a pull request or not. It means finding some way
to manually attach a change log to each patch I send out, which will
be very time consuming. I'd rather not send patches out than jump
through hoops like that, especially when the reason is that the
previous version was ignored.
> b) in June, you said you were going to roll a new version with the
> common OF bits extracted. it's nice to know at the outset that this
> has/hasn't happened
Now if you were following the rest of it, rather than just random parts
that suited you, you'd have noticed that I posted the new version, but
that caused more issues, so I publically said I was reverting back to
the original version.
> Also, prefacing the subject with [PATCH v2] or [PATCH RESEND] lets me
> know there is prior history with the change. Reading the previous
> version is helpful to see what reviewer's concerns were, and whether
> they've been addressed.
Yea, I'm very bad at changing the subject line in that way, and I'm
getting worse at it. Sorry, I try my best, but I can't do better than
that.
>
>
> > It's being
> > posted in the hope that someone will finally either comment on it or
> > merge the damn thing, rather than ignoring it was done when it was
> > last posted.
> >
> >> > drivers/gpu/drm/arm/hdlcd_drv.c | 3 ++-
> >> > drivers/gpu/drm/arm/malidp_drv.c | 4 +++-
> >> > drivers/gpu/drm/armada/armada_drv.c | 2 +-
> >> > drivers/gpu/drm/drm_of.c | 28 +++++++++++++++++++++++--
> >> > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 5 +++--
> >> > drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 7 ++++---
> >> > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 +++-
> >> > drivers/gpu/drm/msm/msm_drv.c | 12 ++++++-----
> >> > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 6 ++++--
> >> > drivers/gpu/drm/sti/sti_drv.c | 5 +++--
> >> > drivers/gpu/drm/sun4i/sun4i_drv.c | 3 ++-
> >> > drivers/gpu/drm/tilcdc/tilcdc_external.c | 4 +++-
> >> > include/drm/drm_of.h | 12 +++++++++++
> >> > 13 files changed, 73 insertions(+), 22 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
> >> > index fb6a418ce6be..6477d1a65266 100644
> >> > --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> >> > +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> >> > @@ -453,7 +453,8 @@ static int hdlcd_probe(struct platform_device *pdev)
> >> > return -EAGAIN;
> >> > }
> >> >
> >> > - component_match_add(&pdev->dev, &match, compare_dev, port);
> >> > + drm_of_component_match_add(&pdev->dev, &match, compare_dev, port);
> >> > + of_node_put(port);
> >>
> >> There's no mention in your commit message about fixing these node leaks.
> >
> > Isn't that kind-of the whole point of this patch?
> >
>
> Not according to the commit msg, it isn't.
>
> You could have just done the of_node_put adds/relocations without
> wrapping component_match_add_release.
No, adding the of_node_put() there without the other changes means that
the OF layer can drop the "port" device node, kfree() it, and reallocate
it as a different device node - which means that we can end up matching
some random other device rather than the intended one. Hence, it is
_fundamentally_ a part of this patch and isn't an independent change.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH 4/4] mtk-mdp: fix compilation warnings if !DEBUG
From: Mauro Carvalho Chehab @ 2016-10-21 13:59 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab, Mauro Carvalho Chehab, Matthias Brugger,
Minghsiu Tsai, Hans Verkuil, linux-arm-kernel, linux-mediatek
In-Reply-To: <cd14afdb178cf490e257368bc899c7a0c690d140.1477058332.git.mchehab@s-opensource.com>
The mtk_mdp_dbg() is empty if !DEBUG. This causes the following
warnings:
drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c: In function ‘mtk_mdp_try_fmt_mplane’:
drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c:231:52: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
org_w, org_h, pix_mp->width, pix_mp->height);
^
drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c: In function ‘mtk_mdp_m2m_start_streaming’:
drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c:414:21: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
ctx->id, ret);
^
With could actually make the code to do something wrong. So,
add an empty block to make it be parsed ok.
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
drivers/media/platform/mtk-mdp/mtk_mdp_core.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
index 2e979f97d1df..848569d4ab90 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
@@ -250,7 +250,7 @@ extern int mtk_mdp_dbg_level;
#else
-#define mtk_mdp_dbg(level, fmt, args...)
+#define mtk_mdp_dbg(level, fmt, args...) {}
#define mtk_mdp_err(fmt, args...)
#define mtk_mdp_dbg_enter()
#define mtk_mdp_dbg_leave()
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox