| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1469677135.29230.7.camel@mtksdaap41>
Date: Thu, 28 Jul 2016 11:38:55 +0800
From: Bibby Hsieh <bibby.hsieh@...iatek.com>
To: Philipp Zabel <p.zabel@...gutronix.de>
CC: David Airlie <airlied@...ux.ie>,
Matthias Brugger <matthias.bgg@...il.com>,
Daniel Vetter <daniel.vetter@...ll.ch>,
<dri-devel@...ts.freedesktop.org>,
<linux-mediatek@...ts.infradead.org>,
Yingjoe Chen <yingjoe.chen@...iatek.com>,
Cawa Cheng <cawa.cheng@...iatek.com>,
Daniel Kurtz <djkurtz@...omium.org>,
YT Shen <yt.shen@...iatek.com>,
Thierry Reding <thierry.reding@...il.com>,
CK Hu <ck.hu@...iatek.com>, Mao Huang <littlecvr@...omium.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>,
"Sascha Hauer" <kernel@...gutronix.de>,
Junzhi Zhao <junzhi.zhao@...iatek.com>
Subject: Re: [PATCH v2 3/3] drm/mediatek: fix the wrong pixel clock when
resolution is 4K
Hi, Philipp,
Thanks for your comments.
On Wed, 2016-07-27 at 11:23 +0200, Philipp Zabel wrote:
> Am Mittwoch, den 27.07.2016, 16:31 +0800 schrieb Bibby Hsieh:
> > From: Junzhi Zhao <junzhi.zhao@...iatek.com>
> >
> > Pixel clock should be 297MHz when resolution is 4K.
>
> This patch does two different things, please don't conflate them.
>
> First, it adds support for 16*3 and 2*3 factors between PLL and pixel
> clock. This should be a separate patch and does what this patch
> description says.
>
> The second part moves the clocks into an array and adds an indirection
> to clock registration depending on of_match_node. This should be a
> separate patch. I suppose this change is in preparation for mt2701
> support, but I'm not sure yet if it is needed at all.
>
Ok, I will move the change for mt2701 to another patch and send it until
mt2701 start upstream.
> regards
> Philipp
>
> > Signed-off-by: Junzhi Zhao <junzhi.zhao@...iatek.com>
> > Signed-off-by: Bibby Hsieh <bibby.hsieh@...iatek.com>
> > ---
> > drivers/gpu/drm/mediatek/mtk_dpi.c | 149 +++++++++++++++++++++++-------------
> > 1 file changed, 96 insertions(+), 53 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > index d05ca79..fa390e0 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > @@ -60,14 +60,25 @@ enum mtk_dpi_out_color_format {
> > MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL
> > };
> >
> > +enum mtk_dpi_clk_id {
> > + MTK_DPI_CLK_DPI_ENGINE,
> > + MTK_DPI_CLK_DPI_PIXEL,
> > + MTK_DPI_CLK_TVD_PLL,
> > + MTK_DPI_CLK_COUNT,
> > +};
> > +
> > +static const char * const mtk_dpi_clk_names[MTK_DPI_CLK_COUNT] = {
> > + [MTK_DPI_CLK_DPI_ENGINE] = "engine",
> > + [MTK_DPI_CLK_DPI_PIXEL] = "pixel",
> > + [MTK_DPI_CLK_TVD_PLL] = "pll",
> > +};
> > +
> > struct mtk_dpi {
> > struct mtk_ddp_comp ddp_comp;
> > struct drm_encoder encoder;
> > void __iomem *regs;
> > struct device *dev;
> > - struct clk *engine_clk;
> > - struct clk *pixel_clk;
> > - struct clk *tvd_clk;
> > + struct clk *clk[MTK_DPI_CLK_COUNT];
> > int irq;
> > struct drm_display_mode mode;
> > enum mtk_dpi_out_color_format color_format;
> > @@ -76,6 +87,7 @@ struct mtk_dpi {
> > enum mtk_dpi_out_channel_swap channel_swap;
> > bool power_sta;
> > u8 power_ctl;
> > + void *data;
> > };
> >
> > static inline struct mtk_dpi *mtk_dpi_from_encoder(struct drm_encoder *e)
> > @@ -114,6 +126,10 @@ struct mtk_dpi_yc_limit {
> > u16 c_bottom;
> > };
> >
> > +struct mtk_dpi_conf {
> > + int (*clk_config)(struct mtk_dpi *dpi, struct drm_display_mode *mode);
> > +};
> > +
> > static void mtk_dpi_mask(struct mtk_dpi *dpi, u32 offset, u32 val, u32 mask)
> > {
> > u32 tmp = readl(dpi->regs + offset) & ~mask;
> > @@ -377,8 +393,8 @@ static void mtk_dpi_power_off(struct mtk_dpi *dpi, enum mtk_dpi_power_ctl pctl)
> > return;
> >
> > mtk_dpi_disable(dpi);
> > - clk_disable_unprepare(dpi->pixel_clk);
> > - clk_disable_unprepare(dpi->engine_clk);
> > + clk_disable_unprepare(dpi->clk[MTK_DPI_CLK_DPI_PIXEL]);
> > + clk_disable_unprepare(dpi->clk[MTK_DPI_CLK_DPI_ENGINE]);
> > dpi->power_sta = false;
> > }
> >
> > @@ -395,13 +411,13 @@ static int mtk_dpi_power_on(struct mtk_dpi *dpi, enum mtk_dpi_power_ctl pctl)
> > if (dpi->power_sta)
> > return 0;
> >
> > - ret = clk_prepare_enable(dpi->engine_clk);
> > + ret = clk_prepare_enable(dpi->clk[MTK_DPI_CLK_DPI_ENGINE]);
> > if (ret) {
> > dev_err(dpi->dev, "Failed to enable engine clock: %d\n", ret);
> > goto err_eng;
> > }
> >
> > - ret = clk_prepare_enable(dpi->pixel_clk);
> > + ret = clk_prepare_enable(dpi->clk[MTK_DPI_CLK_DPI_PIXEL]);
> > if (ret) {
> > dev_err(dpi->dev, "Failed to enable pixel clock: %d\n", ret);
> > goto err_pixel;
> > @@ -412,7 +428,7 @@ static int mtk_dpi_power_on(struct mtk_dpi *dpi, enum mtk_dpi_power_ctl pctl)
> > return 0;
> >
> > err_pixel:
> > - clk_disable_unprepare(dpi->engine_clk);
> > + clk_disable_unprepare(dpi->clk[MTK_DPI_CLK_DPI_ENGINE]);
> > err_eng:
> > dpi->power_ctl &= ~pctl;
> > return ret;
> > @@ -428,34 +444,18 @@ static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi,
> > struct mtk_dpi_sync_param vsync_leven = { 0 };
> > struct mtk_dpi_sync_param vsync_rodd = { 0 };
> > struct mtk_dpi_sync_param vsync_reven = { 0 };
> > - unsigned long pix_rate;
> > - unsigned long pll_rate;
> > - unsigned int factor;
> > + struct mtk_dpi_conf *conf;
> > + int ret;
> >
> > if (!dpi) {
> > dev_err(dpi->dev, "invalid argument\n");
> > return -EINVAL;
> > }
> >
> > - pix_rate = 1000UL * mode->clock;
> > - if (mode->clock <= 74000)
> > - factor = 8 * 3;
> > - else
> > - factor = 4 * 3;
> > - pll_rate = pix_rate * factor;
> > -
> > - dev_dbg(dpi->dev, "Want PLL %lu Hz, pixel clock %lu Hz\n",
> > - pll_rate, pix_rate);
> > -
> > - clk_set_rate(dpi->tvd_clk, pll_rate);
> > - pll_rate = clk_get_rate(dpi->tvd_clk);
> > -
> > - pix_rate = pll_rate / factor;
> > - clk_set_rate(dpi->pixel_clk, pix_rate);
> > - pix_rate = clk_get_rate(dpi->pixel_clk);
> > -
> > - dev_dbg(dpi->dev, "Got PLL %lu Hz, pixel clock %lu Hz\n",
> > - pll_rate, pix_rate);
> > + conf = (struct mtk_dpi_conf *)dpi->data;
> > + ret = conf->clk_config(dpi, mode);
> > + if (ret)
> > + dev_err(dpi->dev, "dpi clk set fail\n");
> >
> > limit.c_bottom = 0x0010;
> > limit.c_top = 0x0FE0;
> > @@ -656,20 +656,83 @@ static const struct component_ops mtk_dpi_component_ops = {
> > .unbind = mtk_dpi_unbind,
> > };
> >
> > +static int mtk_dpi_parse_clk_from_dt(struct mtk_dpi *dpi,
> > + struct device_node *np)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(mtk_dpi_clk_names); i++) {
> > + dpi->clk[i] = of_clk_get_by_name(np,
> > + mtk_dpi_clk_names[i]);
> > + if (IS_ERR(dpi->clk[i]))
> > + return PTR_ERR(dpi->clk[i]);
> > + }
> > + return 0;
> > +}
> > +
> > +static int mt8173_clk_config(struct mtk_dpi *dpi, struct drm_display_mode *mode)
> > +{
> > + unsigned long pix_rate;
> > + unsigned long pll_rate;
> > + unsigned int factor;
> > +
> > + pix_rate = 1000UL * mode->clock;
> > + if (mode->clock <= 27000)
> > + factor = 16 * 3;
> > + else if (mode->clock <= 74250)
> > + factor = 8 * 3;
> > + else if (mode->clock <= 167000)
> > + factor = 4 * 3;
> > + else
> > + factor = 2 * 3;
> > + pll_rate = pix_rate * factor;
> > +
> > + dev_dbg(dpi->dev, "Want PLL %lu Hz, pixel clock %lu Hz\n",
> > + pll_rate, pix_rate);
> > +
> > + pll_rate = clk_get_rate(dpi->clk[MTK_DPI_CLK_TVD_PLL]);
> > + pix_rate = pll_rate / factor;
> > + clk_set_rate(dpi->clk[MTK_DPI_CLK_DPI_PIXEL], pix_rate);
> > + dev_dbg(dpi->dev, "Got PLL %lu Hz, pixel clock %lu Hz\n",
> > + pll_rate, pix_rate);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct mtk_dpi_conf mt8173_conf = {
> > + .clk_config = mt8173_clk_config,
> > +};
> > +
> > +static const struct of_device_id mtk_dpi_of_ids[] = {
> > + { .compatible = "mediatek,mt8173-dpi",
> > + .data = &mt8173_conf,
> > + },
> > + {}
> > +};
> > +
> > static int mtk_dpi_probe(struct platform_device *pdev)
> > {
> > struct device *dev = &pdev->dev;
> > struct mtk_dpi *dpi;
> > struct resource *mem;
> > + struct device_node *np = dev->of_node;
> > struct device_node *ep, *bridge_node = NULL;
> > int comp_id;
> > + const struct of_device_id *match;
> > + struct mtk_dpi_conf *conf;
> > int ret;
> >
> > + match = of_match_node(mtk_dpi_of_ids, dev->of_node);
> > + if (!match)
> > + return -ENODEV;
> > +
> > dpi = devm_kzalloc(dev, sizeof(*dpi), GFP_KERNEL);
> > if (!dpi)
> > return -ENOMEM;
> >
> > dpi->dev = dev;
> > + dpi->data = (void *)match->data;
> > + conf = (struct mtk_dpi_conf *)match->data;
> >
> > mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > dpi->regs = devm_ioremap_resource(dev, mem);
> > @@ -679,24 +742,9 @@ static int mtk_dpi_probe(struct platform_device *pdev)
> > return ret;
> > }
> >
> > - dpi->engine_clk = devm_clk_get(dev, "engine");
> > - if (IS_ERR(dpi->engine_clk)) {
> > - ret = PTR_ERR(dpi->engine_clk);
> > - dev_err(dev, "Failed to get engine clock: %d\n", ret);
> > - return ret;
> > - }
> > -
> > - dpi->pixel_clk = devm_clk_get(dev, "pixel");
> > - if (IS_ERR(dpi->pixel_clk)) {
> > - ret = PTR_ERR(dpi->pixel_clk);
> > - dev_err(dev, "Failed to get pixel clock: %d\n", ret);
> > - return ret;
> > - }
> > -
> > - dpi->tvd_clk = devm_clk_get(dev, "pll");
> > - if (IS_ERR(dpi->tvd_clk)) {
> > - ret = PTR_ERR(dpi->tvd_clk);
> > - dev_err(dev, "Failed to get tvdpll clock: %d\n", ret);
> > + ret = mtk_dpi_parse_clk_from_dt(dpi, np);
> > + if (ret) {
> > + dev_err(dev, "parse tvd div clk failed!");
> > return ret;
> > }
> >
> > @@ -754,11 +802,6 @@ static int mtk_dpi_remove(struct platform_device *pdev)
> > return 0;
> > }
> >
> > -static const struct of_device_id mtk_dpi_of_ids[] = {
> > - { .compatible = "mediatek,mt8173-dpi", },
> > - {}
> > -};
> > -
> > struct platform_driver mtk_dpi_driver = {
> > .probe = mtk_dpi_probe,
> > .remove = mtk_dpi_remove,
>
>
--
Bibby
Powered by blists - more mailing lists