[Intel-gfx] AVI infoframes: default aspect ratio/VIC for CEA modes

2013-11-21 Thread Ville Syrjälä
On Thu, Nov 21, 2013 at 02:48:55AM +, Gohad, Tushar wrote:
> > On Wed, Nov 20, 2013 at 11:45:03PM +, Gohad, Tushar wrote:
> > > > > On Wed, Nov 20, 2013 at 09:48:26PM +, Gohad, Tushar wrote:
> > > > > > Folks,
> > > > > >
> > > > > > When filling in an HDMI AVI infoframe, how does one correctly
> > > > > > determine the "default" picture aspect ratio (and VIC) for CEA
> > > > > > modes that support multiple (4:3 and 16:9) aspect ratios.
> > > > > > 720x576p for example, corresponds to VIC 17 or 18:
> > > > > >
> > > > > > /* 17 - 720x576 at 50Hz */
> > > > > > { DRM_MODE("720x576", DRM_MODE_TYPE_DRIVER, 27000, 720,
> > > > 732,
> > > > > >796, 864, 0, 576, 581, 586, 625, 0,
> > > > > >DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
> > > > > >   .vrefresh = 50, },
> > > > > > /* 18 - 720x576 at 50Hz */
> > > > > > { DRM_MODE("720x576", DRM_MODE_TYPE_DRIVER, 27000, 720,
> > > > 732,
> > > > > >796, 864, 0, 576, 581, 586, 625, 0,
> > > > > >DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
> > > > > >   .vrefresh = 50, },
> > > > > >
> > > > > > Should the "picture aspect ratio" information be derived from
> > > > > > sink EDID (from detailed/cvt/standard timings)? .. possibly in
> > > > > > drm_add_edid_modes() and store the picture aspect ratio in
> > > > > > drm_display_mode perhaps, for later use?  Trying to get a better
> > > > > > understanding of how this usually works.
> > > > >
> > > > > Oh no! someone finally discovered it! Yes, we are totally missing
> > > > > the picture aspect ratio information from the CEA modes. It's been
> > > > > on my TODO list for a couple of month but not exactly high
> > > > > priority. If we want to get a stab a it, we'll review the patches
> > > > > :)
> > >
> 
> > > Thanks Damien.  Sure, I can come up with something quick.  Is the idea
> > > then to store aspect ratio information in drm_display_mode, possibly
> > > as a separate member or as a hint that's part of mode->flags?
> > 
> > It seems natural to extend those flags to describe the picture aspect ratio 
> > (that
> > why dri-devel is in Cc., touching core DRM).
> 
> Cc: dri-devel
> 
> To start with we can use a single bit in drm_display_mode->flags to 
> distinguish 16:9 vs 4:3.

I must admit I haven't really looked into the matter, but do we actually
need any mode bits? Would it be enoguh to just have a property that
allows the user to specify the picture aspect ratio?

-- 
Ville Syrj?l?
Intel OTC


[Intel-gfx] AVI infoframes: default aspect ratio/VIC for CEA modes

2013-11-21 Thread Damien Lespiau
On Thu, Nov 21, 2013 at 07:19:45PM +0200, Ville Syrj?l? wrote:
> > > It seems natural to extend those flags to describe the picture aspect 
> > > ratio (that
> > > why dri-devel is in Cc., touching core DRM).
> > 
> > To start with we can use a single bit in drm_display_mode->flags to 
> > distinguish 16:9 vs 4:3.
> 
> I must admit I haven't really looked into the matter, but do we actually
> need any mode bits? Would it be enoguh to just have a property that
> allows the user to specify the picture aspect ratio?

Well, that's a good question! the picture aspect ratio is attached to
modes in CEA-861, but it could be a good idea to have an orthogonal
picture a-r property on the crtc.

Not quite sure at this point, needs a bit more understanding of the
problem.

-- 
Damien


[Intel-gfx] AVI infoframes: default aspect ratio/VIC for CEA modes

2013-11-21 Thread Gohad, Tushar
> On Wed, Nov 20, 2013 at 11:45:03PM +, Gohad, Tushar wrote:
> > > > On Wed, Nov 20, 2013 at 09:48:26PM +, Gohad, Tushar wrote:
> > > > > Folks,
> > > > >
> > > > > When filling in an HDMI AVI infoframe, how does one correctly
> > > > > determine the "default" picture aspect ratio (and VIC) for CEA
> > > > > modes that support multiple (4:3 and 16:9) aspect ratios.
> > > > > 720x576p for example, corresponds to VIC 17 or 18:
> > > > >
> > > > > /* 17 - 720x576 at 50Hz */
> > > > > { DRM_MODE("720x576", DRM_MODE_TYPE_DRIVER, 27000, 720,
> > > 732,
> > > > >796, 864, 0, 576, 581, 586, 625, 0,
> > > > >DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
> > > > >   .vrefresh = 50, },
> > > > > /* 18 - 720x576 at 50Hz */
> > > > > { DRM_MODE("720x576", DRM_MODE_TYPE_DRIVER, 27000, 720,
> > > 732,
> > > > >796, 864, 0, 576, 581, 586, 625, 0,
> > > > >DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
> > > > >   .vrefresh = 50, },
> > > > >
> > > > > Should the "picture aspect ratio" information be derived from
> > > > > sink EDID (from detailed/cvt/standard timings)? .. possibly in
> > > > > drm_add_edid_modes() and store the picture aspect ratio in
> > > > > drm_display_mode perhaps, for later use?  Trying to get a better
> > > > > understanding of how this usually works.
> > > >
> > > > Oh no! someone finally discovered it! Yes, we are totally missing
> > > > the picture aspect ratio information from the CEA modes. It's been
> > > > on my TODO list for a couple of month but not exactly high
> > > > priority. If we want to get a stab a it, we'll review the patches
> > > > :)
> >

> > Thanks Damien.  Sure, I can come up with something quick.  Is the idea
> > then to store aspect ratio information in drm_display_mode, possibly
> > as a separate member or as a hint that's part of mode->flags?
> 
> It seems natural to extend those flags to describe the picture aspect ratio 
> (that
> why dri-devel is in Cc., touching core DRM).

Cc: dri-devel

To start with we can use a single bit in drm_display_mode->flags to distinguish 
16:9 vs 4:3.


> 
> > > I realized that I did not actually answer the question. The CEA
> > > modes come with their defined aspect ratio, it's part of the CEA 861
> > > standard.  In this case VIC 17 is 4:3, VIC 18 is 16:9.
> > >
> >
> > I understand the VIC/aspect ratio correlation, however in order to set
> > the VIC correctly, don't we need to first determine the picture aspect
> > ratio?  My question really is, how do we "select" the aspect ratio for
> > the ambiguous cases, such as 720x576 which neither truly 4:3 nor 16:9?
> 
> If we expose both to user-space (through the flags), user-space is responsible
> for picking one. One detail (out of many, I'm sure) is that, today, we accept
> modes without any picture aspect ratio information. We need to keep that
> behaviour working.

Exactly the reason for the term "default aspect ratio" in original question.  
:)   I didn't see a way to set/force an aspect ratio via KMS or other methods 
(xrandr).  Is this for historical reasons?


> > I see that "drm_add_edid_modes()" enumerates detailed, CVT, standard,
> > established, CEA modes.  Should picture aspect ratio determination be
> > a part of that process, based on the EDID info?
> 
> Right, so I guess you'd need to add the proper flags to the modes as you're
> parsing them. May prove a bit tricky.
>

Okay, so we seem to agree that we should set the default picture aspect ratio 
as we are parsing the EDID timings info.  

Comments from other folks on any pitfalls of this approach?


> > Section 7.2.2 of CEA-861-D spec reads -
> >
> > "7.2.2 Order of Dual-Aspect Ratio Detailed Timing Descriptors
> 
> > Source devices that do not support the AVI InfoFrame (e.g., DVI
> > sources) shall consider the first EDID descriptor of any dual-aspect
> > ratio timing to be the display-assumed aspect ratio for that timing
> > ..."
> >
> > How do we figure out the "display-assumed aspect ratio"?
> 
> It seems that CEA-861-E rewords that sentence:
> 
> "A sink not capable of receiving AVI InfoFrames shall only declare video
> formats with different video timings in its EDID data structure unless the 
> sink
> declares it is capable of displaying a video timing in either picture aspect
> ratio."
> 
> This relates to 4.1:
> 
> "For a display device to simultaneously support both formats, the source
> needs a way to let the display device know the picture aspect ratio in which
> the video should be displayed. A DTV shall list only one picture aspect ratio 
> of
> any dual-aspect ratio timing unless it is capable of receiving and decoding 
> the
> AVI InfoFrame defined in Section 6."
> 
> However, it is possible for a DTV that has no support for the AVI InfoFrame to
> still support both aspect ratios of such formats as a user programmable 
> option.
> In that case, the EDID Detailed Timing