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

2018-02-27 Thread Laurent Pinchart 
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 
> Signed-off-by: Jyri Sarha 
> ---
>  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_REGSSZ_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

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

2018-02-19 Thread Tomi Valkeinen 
Hi Jyri,

On 16/02/18 13:25, 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 
> Signed-off-by: Jyri Sarha 
> ---
>  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_REGSSZ_512
>  
>  struct dss_device {
>   struct platform_device *pdev;
> - void __iomem*base;
> + void __iomem*base;

Extra changes here, and in the above SZ_REGS.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel