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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1415194882.5696.9.camel@pengutronix.de>
Date:	Wed, 05 Nov 2014 14:41:22 +0100
From:	Philipp Zabel <p.zabel@...gutronix.de>
To:	Andy Yan <andy.yan@...k-chips.com>
Cc:	airlied@...ux.ie, heiko@...ech.de, fabio.estevam@...escale.com,
	rmk+kernel@....linux.org.uk,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Grant Likely <grant.likely@...aro.org>,
	Rob Herring <robh+dt@...nel.org>,
	Shawn Guo <shawn.guo@...aro.org>,
	Josh Boyer <jwboyer@...hat.com>,
	Sean Paul <seanpaul@...omium.org>,
	Inki Dae <inki.dae@...sung.com>,
	Dave Airlie <airlied@...hat.com>,
	Arnd Bergmann <arnd@...db.de>,
	Lucas Stach <l.stach@...gutronix.de>,
	Zubair.Kakakhel@...tec.com, djkurtz@...gle.com, ykk@...k-chips.com,
	linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
	devel@...verdev.osuosl.org, devicetree@...r.kernel.org,
	linux-rockchip@...ts.infradead.org
Subject: Re: [PATCH 1/2] imx-drm: imx-hdmi: split imx soc specific code from
 imx-hdmi

Hi Andy,

I think separating the core from the SoC specific part is a good step
into the right direction.

Am Mittwoch, den 05.11.2014, 20:55 +0800 schrieb Andy Yan:
> imx6 and rockchip rk3288 and JZ4780 (Ingenic Xburst/MIPS)
> use the interface compatible Designware HDMI IP, but they
> also have some lightly difference, such as phy pll configuration,
> register width(imx hdmi register is one byte, but rk3288 is 4
> bytes width), 4K support(imx6 doesn't support 4k, but rk3288 does),
> clk useage,and the crtc mux configuration is also platform specific.
> 
> To reuse the imx hdmi driver, split the platform specific code out
> to dw_hdmi-imx.c.
> 
> Signed-off-by: Andy Yan <andy.yan@...k-chips.com>
[...]
> +static int imx_hdmi_parse_dt(struct imx_hdmi_priv *hdmi)
> +{
> +	struct device_node *np = hdmi->dev->of_node;
> +
> +	hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "gpr");
> +	if (IS_ERR(hdmi->regmap)) {
> +		dev_err(hdmi->dev, "Unable to get gpr\n");
> +		return PTR_ERR(hdmi->regmap);
> +	}
> +
> +	hdmi->isfr_clk = devm_clk_get(hdmi->dev, "isfr");
> +	if (IS_ERR(hdmi->isfr_clk)) {
> +		dev_err(hdmi->dev, "Unable to get HDMI isfr clk\n");
> +		return PTR_ERR(hdmi->isfr_clk);
> +	}
> +
> +	hdmi->iahb_clk = devm_clk_get(hdmi->dev, "iahb");
> +	if (IS_ERR(hdmi->iahb_clk)) {
> +		dev_err(hdmi->dev, "Unable to get HDMI iahb clk\n");
> +		return PTR_ERR(hdmi->iahb_clk);
> +	}

Surely this IP core needs to be clocked regardless of the SoC? How are
clocks controlled on rk3288 and jz4780?

[...]
> +/*On rockchip platform, no-word access to the hdmi
> + * register will causes an imprecise external abort
> + */
> +static inline void hdmi_writel(struct imx_hdmi *hdmi, u32 val, int offset)
> +{
> +	writel(val, hdmi->regs + (offset << 2));
> +}
>  
> -	bool phy_enabled;
> -	struct drm_display_mode previous_mode;
> +static inline u32 hdmi_readl(struct imx_hdmi *hdmi, int offset)
> +{
> +	return readl(hdmi->regs + (offset << 2));
> +}
>  
> -	struct regmap *regmap;
> -	struct i2c_adapter *ddc;
> -	void __iomem *regs;
> +static void hdmi_modl(struct imx_hdmi *hdmi, u32 data, u32 mask, unsigned reg)
> +{
> +	u32 val = hdmi_readl(hdmi, reg) & ~mask;
>  
> -	unsigned int sample_rate;
> -	int ratio;
> -};
> +	val |= data & mask;
> +	hdmi_writel(hdmi, val, reg);
> +}
>  
> -static void imx_hdmi_set_ipu_di_mux(struct imx_hdmi *hdmi, int ipu_di)
> +static void hdmi_mask_writel(struct imx_hdmi *hdmi, u32 data, unsigned int reg,
> +			     u32 shift, u32 mask)
>  {
> -	regmap_update_bits(hdmi->regmap, IOMUXC_GPR3,
> -			   IMX6Q_GPR3_HDMI_MUX_CTL_MASK,
> -			   ipu_di << IMX6Q_GPR3_HDMI_MUX_CTL_SHIFT);
> +	hdmi_modl(hdmi, data << shift, mask, reg);
>  }
>  
> -static inline void hdmi_writeb(struct imx_hdmi *hdmi, u8 val, int offset)
> +static inline void hdmi_writeb(struct imx_hdmi *hdmi, u32 val, int offset)
>  {
>  	writeb(val, hdmi->regs + offset);
>  }
>  
> -static inline u8 hdmi_readb(struct imx_hdmi *hdmi, int offset)
> +static inline u32 hdmi_readb(struct imx_hdmi *hdmi, int offset)
>  {
>  	return readb(hdmi->regs + offset);
>  }
>  
> -static void hdmi_modb(struct imx_hdmi *hdmi, u8 data, u8 mask, unsigned reg)
> +static void hdmi_modb(struct imx_hdmi *hdmi, u32 data, u32 mask, unsigned reg)
>  {
>  	u8 val = hdmi_readb(hdmi, reg) & ~mask;

Do you really need to use readl instead of readb? In my opinion it would
be better then to convert the driver to use regmap for register access
(in a separate patch) and then let the SoC specific driver extension
provide the regmap to the core driver.

[...]
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> new file mode 100644
> index 0000000..e7e8285
> --- /dev/null
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -0,0 +1,114 @@
> +/*
> + * Copyright (C) 2011 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#ifndef __DW_HDMI_H__
> +#define __DW_HDMI_H__
> +
> +#include <drm/drmP.h>
> +
> +#define HDMI_EDID_LEN           512
> +
> +enum {
> +	RES_8,
> +	RES_10,
> +	RES_12,
> +	RES_MAX,
> +};
> +
> +enum imx_hdmi_devtype {
> +	IMX6Q_HDMI,
> +	IMX6DL_HDMI,
> +};
> +
> +struct mpll_config {
> +	unsigned long mpixelclock;
> +	struct {
> +		u16 cpce;
> +		u16 gmp;
> +	} res[RES_MAX];
> +};
> +
> +struct curr_ctrl {
> +	unsigned long mpixelclock;
> +	u16 curr[RES_MAX];
> +};

For a header file in include/drm/bridge maybe these struct names are a
bit too generic now.

regards
Philipp


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ