Hi Kamil,
I like the solution as well. Two suggestions to consider below.
On Fri, May 16, 2014 at 9:03 PM, Kamil Debski <k.debski@xxxxxxxxxxx> wrote:
> MFC versions support a different set of formats, this specially applies
> to the raw YUV formats. This patch changes enum_fmt, so that it only
> reports formats that are supported by the used MFC version.
>
> Signed-off-by: Kamil Debski <k.debski@xxxxxxxxxxx>
> ---
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> index 9370c34..d5efb10 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> @@ -223,6 +223,7 @@ struct s5p_mfc_buf_align {
> struct s5p_mfc_variant {
> unsigned int version;
> unsigned int port_num;
> + u32 version_bit;
> struct s5p_mfc_buf_size *buf_size;
> struct s5p_mfc_buf_align *buf_align;
> char *fw_name;
> @@ -666,6 +667,7 @@ struct s5p_mfc_fmt {
> u32 codec_mode;
> enum s5p_mfc_fmt_type type;
> u32 num_planes;
> + u32 versions;
> };
>
> /**
> @@ -705,4 +707,9 @@ void set_work_bit_irqsave(struct s5p_mfc_ctx *ctx);
> #define IS_MFCV6_PLUS(dev) (dev->variant->version >= 0x60 ? 1 : 0)
> #define IS_MFCV7_PLUS(dev) (dev->variant->version >= 0x70 ? 1 : 0)
>
> +#define MFC_V5 BIT(0)
> +#define MFC_V6 BIT(1)
> +#define MFC_V7 BIT(2)
These may be confusing. I'd suggest at least suffixing those macros with _BIT.
Or better yet, please make this into an enum and also make
variant->versions of size BITS_TO_LONGS() with max enum value.
> /* Get format */
> @@ -384,11 +402,9 @@ static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f)
> mfc_err("Unknown codec\n");
> return -EINVAL;
> }
> - if (!IS_MFCV6_PLUS(dev)) {
> - if (fmt->fourcc == V4L2_PIX_FMT_VP8) {
> - mfc_err("Not supported format.\n");
> - return -EINVAL;
> - }
> + if ((dev->variant->version_bit & fmt->versions) == 0) {
> + mfc_err("Unsupported format by this MFC version.\n");
> + return -EINVAL;
What do you think of moving this check to find_format()? You wouldn't
have to duplicate it across enum_fmt and try_fmt then...
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
