Re: [Intel-gfx] [PATCH 06/12] video/hdmi: Derive the bar data valid bit from the bar data fields
On Thu, Aug 15, 2013 at 04:45:56PM +0200, Thierry Reding wrote: > On Wed, Aug 14, 2013 at 06:19:09PM +0100, Damien Lespiau wrote: > > Just like: > > > > Author: Damien Lespiau > > Date: Mon Aug 12 11:53:24 2013 +0100 > > > > video/hdmi: Don't let the user of this API create invalid infoframes > > > > But this time for the horizontal/vertical bar data present bits. > > > > Signed-off-by: Damien Lespiau > > Reviewed-by: Ville Syrjälä > > --- > > drivers/video/hdmi.c | 5 +++-- > > include/linux/hdmi.h | 2 -- > > 2 files changed, 3 insertions(+), 4 deletions(-) > > Same comments as for patch 5. Although I begin to see some sense in > this. Perhaps not exposing these boolean fields is a good idea after > all. I wonder if we're excluding some particular use-case by not > exposing these fields. Right, so I included patch 5/6 to the v4 of the series, putting preventing someone from generating invalid infoframes before the hypothetical use-case where we want the detail of the fields. We are, de-facto, excluding the case where we'd want to have _unpack() variants decoding infoframes, but that would only be useful if we wanted to check the validity of infoframes in the kernel for instance. I think it's unlikely we'd like to do that. It's also straightforward to revert the 2 patches if/when that happens. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/12] video/hdmi: Derive the bar data valid bit from the bar data fields
On Wed, Aug 14, 2013 at 06:19:09PM +0100, Damien Lespiau wrote: > Just like: > > Author: Damien Lespiau > Date: Mon Aug 12 11:53:24 2013 +0100 > > video/hdmi: Don't let the user of this API create invalid infoframes > > But this time for the horizontal/vertical bar data present bits. > > Signed-off-by: Damien Lespiau > Reviewed-by: Ville Syrjälä > --- > drivers/video/hdmi.c | 5 +++-- > include/linux/hdmi.h | 2 -- > 2 files changed, 3 insertions(+), 4 deletions(-) Same comments as for patch 5. Although I begin to see some sense in this. Perhaps not exposing these boolean fields is a good idea after all. I wonder if we're excluding some particular use-case by not exposing these fields. Thierry pgpFQLpm9ohml.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 06/12] video/hdmi: Derive the bar data valid bit from the bar data fields
Just like:
Author: Damien Lespiau
Date: Mon Aug 12 11:53:24 2013 +0100
video/hdmi: Don't let the user of this API create invalid infoframes
But this time for the horizontal/vertical bar data present bits.
Signed-off-by: Damien Lespiau
Reviewed-by: Ville Syrjälä
---
drivers/video/hdmi.c | 5 +++--
include/linux/hdmi.h | 2 --
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index e36da36..ac84215 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -101,10 +101,11 @@ ssize_t hdmi_avi_infoframe_pack(struct hdmi_avi_infoframe
*frame, void *buffer,
if (frame->active_aspect & 0xf)
ptr[0] |= BIT(4);
- if (frame->horizontal_bar_valid)
+ /* Bit 3 and 2 indicate if we transmit horizontal/vertical bar data */
+ if (frame->top_bar || frame->bottom_bar)
ptr[0] |= BIT(3);
- if (frame->vertical_bar_valid)
+ if (frame->left_bar || frame->right_bar)
ptr[0] |= BIT(2);
ptr[1] = ((frame->colorimetry & 0x3) << 6) |
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index 931474c6..b98340b 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -109,8 +109,6 @@ struct hdmi_avi_infoframe {
unsigned char version;
unsigned char length;
enum hdmi_colorspace colorspace;
- bool horizontal_bar_valid;
- bool vertical_bar_valid;
enum hdmi_scan_mode scan_mode;
enum hdmi_colorimetry colorimetry;
enum hdmi_picture_aspect picture_aspect;
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/12] video/hdmi: Derive the bar data valid bit from the bar data fields
On Wed, Aug 14, 2013 at 12:17:22AM +0100, Damien Lespiau wrote:
> Just like:
>
> Author: Damien Lespiau
> Date: Mon Aug 12 11:53:24 2013 +0100
>
> video/hdmi: Don't let the user of this API create invalid infoframes
>
> But this time for the horizontal/vertical bar data present bits.
>
> Signed-off-by: Damien Lespiau
> ---
> drivers/video/hdmi.c | 5 +++--
> include/linux/hdmi.h | 2 --
> 2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index e36da36..ac84215 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -101,10 +101,11 @@ ssize_t hdmi_avi_infoframe_pack(struct
> hdmi_avi_infoframe *frame, void *buffer,
> if (frame->active_aspect & 0xf)
> ptr[0] |= BIT(4);
>
> - if (frame->horizontal_bar_valid)
> + /* Bit 3 and 2 indicate if we transmit horizontal/vertical bar data */
> + if (frame->top_bar || frame->bottom_bar)
> ptr[0] |= BIT(3);
>
> - if (frame->vertical_bar_valid)
> + if (frame->left_bar || frame->right_bar)
> ptr[0] |= BIT(2);
Technically top=0,bottom=0 or left=0,right=0 is a valid bar setup,
but it would indicate that the entire picture is made up of the
bottom/right bar. I guess no one would really want to use such a
setup, and even if they do, they could just use some N!=0 for
both bars to achieve the same effect. So we don't seem to lose
anything by doing this.
Reviewed-by: Ville Syrjälä
>
> ptr[1] = ((frame->colorimetry & 0x3) << 6) |
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index 931474c6..b98340b 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -109,8 +109,6 @@ struct hdmi_avi_infoframe {
> unsigned char version;
> unsigned char length;
> enum hdmi_colorspace colorspace;
> - bool horizontal_bar_valid;
> - bool vertical_bar_valid;
> enum hdmi_scan_mode scan_mode;
> enum hdmi_colorimetry colorimetry;
> enum hdmi_picture_aspect picture_aspect;
> --
> 1.8.3.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 06/12] video/hdmi: Derive the bar data valid bit from the bar data fields
Just like:
Author: Damien Lespiau
Date: Mon Aug 12 11:53:24 2013 +0100
video/hdmi: Don't let the user of this API create invalid infoframes
But this time for the horizontal/vertical bar data present bits.
Signed-off-by: Damien Lespiau
---
drivers/video/hdmi.c | 5 +++--
include/linux/hdmi.h | 2 --
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index e36da36..ac84215 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -101,10 +101,11 @@ ssize_t hdmi_avi_infoframe_pack(struct hdmi_avi_infoframe
*frame, void *buffer,
if (frame->active_aspect & 0xf)
ptr[0] |= BIT(4);
- if (frame->horizontal_bar_valid)
+ /* Bit 3 and 2 indicate if we transmit horizontal/vertical bar data */
+ if (frame->top_bar || frame->bottom_bar)
ptr[0] |= BIT(3);
- if (frame->vertical_bar_valid)
+ if (frame->left_bar || frame->right_bar)
ptr[0] |= BIT(2);
ptr[1] = ((frame->colorimetry & 0x3) << 6) |
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index 931474c6..b98340b 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -109,8 +109,6 @@ struct hdmi_avi_infoframe {
unsigned char version;
unsigned char length;
enum hdmi_colorspace colorspace;
- bool horizontal_bar_valid;
- bool vertical_bar_valid;
enum hdmi_scan_mode scan_mode;
enum hdmi_colorimetry colorimetry;
enum hdmi_picture_aspect picture_aspect;
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
