Hi,
On Wed, 2010-09-08 at 13:17 +0200, ext Archit Taneja wrote:
> Add dss_features.c and dss_features.h for the dss_features framework
>
> Signed-off-by: Archit Taneja <[email protected]>
> ---
> drivers/video/omap2/dss/dss_features.c | 171
> ++++++++++++++++++++++++++++++++
> drivers/video/omap2/dss/dss_features.h | 51 ++++++++++
> 2 files changed, 222 insertions(+), 0 deletions(-)
> create mode 100644 drivers/video/omap2/dss/dss_features.c
> create mode 100644 drivers/video/omap2/dss/dss_features.h
>
> diff --git a/drivers/video/omap2/dss/dss_features.c
> b/drivers/video/omap2/dss/dss_features.c
> new file mode 100644
> index 0000000..e87816f
> --- /dev/null
> +++ b/drivers/video/omap2/dss/dss_features.c
> @@ -0,0 +1,171 @@
> +/*
> + * linux/drivers/video/omap2/dss/dss_features.c
> + *
> + * Copyright (C) 2010 Texas Instruments
> + * Author: Archit Taneja <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +
> +#include <plat/display.h>
> +#include <plat/cpu.h>
> +
> +#include "dss_features.h"
> +
> +/* Defines a generic omap register field */
> +struct dss_reg_field {
> + enum dss_feat_reg_field id;
> + u8 start, end;
> +};
> +
> +struct omap_dss_features {
> + struct dss_reg_field reg_fields[MAX_DSS_REG_FIELDS];
Perhaps this should be a pointer to a list, and add a separate field for
num_reg_fields. That way you don't need a MAX_DSS_REG_FIELDS definition.
> + u32 has_feature;
> +
> + int num_mgrs;
> + int num_ovls;
> + enum omap_display_type supported_displays[MAX_DSS_MANAGERS];
> + enum omap_color_mode supported_color_modes[MAX_DSS_OVERLAYS];
Perhaps same could be done here.
> +};
> +
> +/* This struct is assigned to one of the below during initialization */
> +static struct omap_dss_features omap_current_dss_features;
> +
> +/* OMAP2 DSS Features */
> +static struct omap_dss_features omap2_dss_features = {
> + .reg_fields = {
> + {FIRHINC, 11, 0},
> + {FIRVINC, 27, 16},
> + {FIFOLOWTHRESHOLD, 8, 0},
> + {FIFOHIGHTHRESHOLD, 24, 16},
> + {FIFOSIZE, 8, 0},
You should have space after { and before }.
> + },
> +
> + .num_mgrs = 2,
> + .num_ovls = 3,
> + .supported_displays = {
> + /* OMAP_DSS_CHANNEL_LCD */
> + OMAP_DISPLAY_TYPE_DPI | OMAP_DISPLAY_TYPE_DBI |
> + OMAP_DISPLAY_TYPE_SDI | OMAP_DISPLAY_TYPE_DSI,
> +
> + /* OMAP_DSS_CHANNEL_DIGIT */
> + OMAP_DISPLAY_TYPE_VENC,
> + },
> + .supported_color_modes = {
> + /* OMAP_DSS_GFX */
> + OMAP_DSS_COLOR_CLUT1 | OMAP_DSS_COLOR_CLUT2 |
> + OMAP_DSS_COLOR_CLUT4 | OMAP_DSS_COLOR_CLUT8 |
> + OMAP_DSS_COLOR_RGB12U | OMAP_DSS_COLOR_RGB16 |
> + OMAP_DSS_COLOR_RGB24U | OMAP_DSS_COLOR_RGB24P,
> +
> + /* OMAP_DSS_VIDEO1 */
> + OMAP_DSS_COLOR_RGB16 | OMAP_DSS_COLOR_RGB24U |
> + OMAP_DSS_COLOR_RGB24P | OMAP_DSS_COLOR_YUV2 |
> + OMAP_DSS_COLOR_UYVY,
> +
> + /* OMAP_DSS_VIDEO2 */
> + OMAP_DSS_COLOR_RGB16 | OMAP_DSS_COLOR_RGB24U |
> + OMAP_DSS_COLOR_RGB24P | OMAP_DSS_COLOR_YUV2 |
> + OMAP_DSS_COLOR_UYVY,
> + },
> +};
> +
> +/* OMAP3 DSS Features */
> +static struct omap_dss_features omap3_dss_features = {
> + .reg_fields = {
> + {FIRHINC, 12, 0},
> + {FIRVINC, 28, 16},
> + {FIFOLOWTHRESHOLD, 11, 0},
> + {FIFOHIGHTHRESHOLD, 27, 16},
> + {FIFOSIZE, 10, 0},
> + },
> + .has_feature = FEAT_GLOBAL_ALPHA,
> +
> + .num_mgrs = 2,
> + .num_ovls = 3,
> + .supported_displays = {
> + /* OMAP_DSS_CHANNEL_LCD */
> + OMAP_DISPLAY_TYPE_DPI | OMAP_DISPLAY_TYPE_DBI |
> + OMAP_DISPLAY_TYPE_SDI | OMAP_DISPLAY_TYPE_DSI,
> +
> + /* OMAP_DSS_CHANNEL_DIGIT */
> + OMAP_DISPLAY_TYPE_VENC,
> + },
> + .supported_color_modes = {
> + /* OMAP_DSS_GFX */
> + OMAP_DSS_COLOR_CLUT1 | OMAP_DSS_COLOR_CLUT2 |
> + OMAP_DSS_COLOR_CLUT4 | OMAP_DSS_COLOR_CLUT8 |
> + OMAP_DSS_COLOR_RGB12U | OMAP_DSS_COLOR_ARGB16 |
> + OMAP_DSS_COLOR_RGB16 | OMAP_DSS_COLOR_RGB24U |
> + OMAP_DSS_COLOR_RGB24P | OMAP_DSS_COLOR_ARGB32 |
> + OMAP_DSS_COLOR_RGBA32 | OMAP_DSS_COLOR_RGBX32,
> +
> + /* OMAP_DSS_VIDEO1 */
> + OMAP_DSS_COLOR_RGB24U | OMAP_DSS_COLOR_RGB24P |
> + OMAP_DSS_COLOR_RGB12U | OMAP_DSS_COLOR_RGB16 |
> + OMAP_DSS_COLOR_YUV2 | OMAP_DSS_COLOR_UYVY,
> +
> + /* OMAP_DSS_VIDEO2 */
> + OMAP_DSS_COLOR_RGB12U | OMAP_DSS_COLOR_ARGB16 |
> + OMAP_DSS_COLOR_RGB16 | OMAP_DSS_COLOR_RGB24U |
> + OMAP_DSS_COLOR_RGB24P | OMAP_DSS_COLOR_YUV2 |
> + OMAP_DSS_COLOR_UYVY | OMAP_DSS_COLOR_ARGB32 |
> + OMAP_DSS_COLOR_RGBA32 | OMAP_DSS_COLOR_RGBX32,
> + },
> +};
> +
> +/* Functions returning values related to a DSS feature */
> +int dss_feat_get_num_mgrs(void)
> +{
> + return omap_current_dss_features.num_mgrs;
> +}
> +
> +int dss_feat_get_num_ovls(void)
> +{
> + return omap_current_dss_features.num_ovls;
> +}
> +
> +enum omap_display_type dss_feat_get_supported_displays(int id)
> +{
> + return omap_current_dss_features.supported_displays[id];
> +}
> +
> +enum omap_color_mode dss_feat_get_supported_color_modes(int id)
> +{
> + return omap_current_dss_features.supported_color_modes[id];
> +}
> +
> +/* DSS has_feature check */
> +bool dss_has_feature(enum dss_feat_id id)
> +{
> + return omap_current_dss_features.has_feature & id;
> +}
> +
> +void dss_feat_get_reg_field(enum dss_feat_reg_field id, u8 *start, u8 *end)
> +{
> + *start = omap_current_dss_features.reg_fields[id].start;
> + *end = omap_current_dss_features.reg_fields[id].end;
> +}
> +
> +void dss_features_init(void)
> +{
> + if (cpu_is_omap24xx())
> + omap_current_dss_features = omap2_dss_features;
> + else
> + omap_current_dss_features = omap3_dss_features;
> +}
> diff --git a/drivers/video/omap2/dss/dss_features.h
> b/drivers/video/omap2/dss/dss_features.h
> new file mode 100644
> index 0000000..f1a7e17
> --- /dev/null
> +++ b/drivers/video/omap2/dss/dss_features.h
> @@ -0,0 +1,51 @@
> +/*
> + * linux/drivers/video/omap2/dss/dss_features.h
> + *
> + * Copyright (C) 2010 Texas Instruments
> + * Author: Archit Taneja <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __OMAP2_DSS_FEATURES_H
> +#define __OMAP2_DSS_FEATURES_H
> +
> +#define MAX_DSS_MANAGERS 2
> +#define MAX_DSS_OVERLAYS 3
> +#define MAX_DSS_REG_FIELDS 10
> +
> +/* DSS has feature id */
> +enum dss_feat_id {
> + FEAT_GLOBAL_ALPHA = 1 << 0,
> + FEAT_GLOBAL_ALPHA_VID1 = 1 << 1,
> +};
> +
> +/* DSS register field id */
> +enum dss_feat_reg_field {
> + FIRHINC,
> + FIRVINC,
> + FIFOHIGHTHRESHOLD,
> + FIFOLOWTHRESHOLD,
> + FIFOSIZE,
> +};
These enums should also have some prefix. FEAT_REG?
> +
> +/* DSS Feature Functions */
> +int dss_feat_get_num_mgrs(void);
> +int dss_feat_get_num_ovls(void);
> +enum omap_display_type dss_feat_get_supported_displays(int id);
> +enum omap_color_mode dss_feat_get_supported_color_modes(int id);
These functions are a bit confusing. I know they are similar to what
already exists in DSS driver for overlays and managers, but...
Using get_num_mgrs() and then looping through then would suggest that
the parameter to get_supported_displays() would be an index to an array
(which it is), but at the same time the parameter is an overlay ID.
Perhaps enum omap_plane and enum omap_channel from display.h could be
used here?
Which reminds me that the names of those enums in display.h should be
also reviewed... A global "omap_channel" is not very descriptive =).
Should perhaps be omap_dss_channel or manager".
Tomi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
