Re: [PATCH RFC 5/9] drm/omap: move common stuff from dss.h to omapdss.h

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Jyri,

Thank you for the patch.

On Friday, 16 February 2018 13:25:06 EET Jyri Sarha wrote:
> The new DSS6 driver needs some structs and defines which are currently
> in dss.h, which is for the old DSS driver.
> 
> Move the required structs and defines from dss.h to omapdss.h.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/dss/dss.h     | 41 ++-----------------------------
>  drivers/gpu/drm/omapdrm/dss/omapdss.h | 37 +++++++++++++++++++++++++++++++
>  2 files changed, 39 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dss.h
> b/drivers/gpu/drm/omapdrm/dss/dss.h index 434262a..fa206ca 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/dss.h
> @@ -70,14 +70,6 @@ struct seq_file;
>  	pr_warn("omapdss: " format, ##__VA_ARGS__)
>  #endif
> 
> -/* OMAP TRM gives bitfields as start:end, where start is the higher bit
> -   number. For example 7:0 */
> -#define FLD_MASK(start, end)	(((1 << ((start) - (end) + 1)) - 1) << 
(end))
> -#define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
> -#define FLD_GET(val, start, end) (((val) & FLD_MASK(start, end)) >> (end))
> -#define FLD_MOD(orig, val, start, end) \
> -	(((orig) & ~FLD_MASK(start, end)) | FLD_VAL(val, start, end))
> -
>  enum dss_model {
>  	DSS_MODEL_OMAP2,
>  	DSS_MODEL_OMAP3,
> @@ -86,12 +78,6 @@ enum dss_model {
>  	DSS_MODEL_DRA7,
>  };
> 
> -enum dss_io_pad_mode {
> -	DSS_IO_PAD_MODE_RESET,
> -	DSS_IO_PAD_MODE_RFBI,
> -	DSS_IO_PAD_MODE_BYPASS,
> -};
> -
>  enum dss_hdmi_venc_clk_source_select {
>  	DSS_VENC_TV_CLK = 0,
>  	DSS_HDMI_M_PCLK = 1,
> @@ -215,34 +201,11 @@ struct dss_reg_field {
>  	u8 start, end;
>  };
> 
> -struct dispc_clock_info {
> -	/* rates that we get with dividers below */
> -	unsigned long lck;
> -	unsigned long pck;
> -
> -	/* dividers */
> -	u16 lck_div;
> -	u16 pck_div;
> -};
> -
> -struct dss_lcd_mgr_config {
> -	enum dss_io_pad_mode io_pad_mode;
> -
> -	bool stallmode;
> -	bool fifohandcheck;
> -
> -	struct dispc_clock_info clock_info;
> -
> -	int video_port_width;
> -
> -	int lcden_sig_polarity;
> -};
> -
> -#define DSS_SZ_REGS			SZ_512
> +#define DSS_SZ_REGS                    SZ_512
> 
>  struct dss_device {
>  	struct platform_device *pdev;
> -	void __iomem    *base;
> +	void __iomem	*base;
>  	struct regmap	*syscon_pll_ctrl;
>  	u32		syscon_pll_ctrl_offset;
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> b/drivers/gpu/drm/omapdrm/dss/omapdss.h index 493237e..9d789c2 100644
> --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> @@ -647,6 +647,43 @@ static inline bool omapdss_device_is_enabled(struct
> omap_dss_device *dssdev) struct omap_dss_device *
>  omapdss_of_find_source_for_first_ep(struct device_node *node);
> 
> +/* OMAP TRM gives bitfields as start:end, where start is the higher bit
> +   number. For example 7:0 */
> +#define FLD_MASK(start, end)	(((1 << ((start) - (end) + 1)) - 1) << 
(end))

While at it would it make sense to use GENMASK to implement this ?

> +#define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
> +#define FLD_GET(val, start, end) (((val) & FLD_MASK(start, end)) >> (end))
> +#define FLD_MOD(orig, val, start, end) \
> +	(((orig) & ~FLD_MASK(start, end)) | FLD_VAL(val, start, end))
> +
> +enum dss_io_pad_mode {
> +	DSS_IO_PAD_MODE_RESET,
> +	DSS_IO_PAD_MODE_RFBI,
> +	DSS_IO_PAD_MODE_BYPASS,
> +};
> +
> +struct dispc_clock_info {
> +	/* rates that we get with dividers below */
> +	unsigned long lck;
> +	unsigned long pck;
> +
> +	/* dividers */
> +	u16 lck_div;
> +	u16 pck_div;
> +};

I think we should start documenting the common structures and enums with 
kerneldoc. omapdss.h and dss.h are a bit of a mess, let's try to make them 
readable :-)

> +struct dss_lcd_mgr_config {
> +	enum dss_io_pad_mode io_pad_mode;
> +
> +	bool stallmode;
> +	bool fifohandcheck;
> +
> +	struct dispc_clock_info clock_info;
> +
> +	int video_port_width;
> +
> +	int lcden_sig_polarity;
> +};
> +
>  struct device_node *dss_of_port_get_parent_device(struct device_node
> *port);
>  u32 dss_of_port_get_port_number(struct device_node *port);

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel