On Fri, Nov 17, 2017 at 08:53:54AM +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 11/16/2017 9:56 PM, Ville Syrjälä wrote:
> > On Thu, Nov 16, 2017 at 08:31:36PM +0530, Sharma, Shashank wrote:
> >> Regards
> >>
> >> Shashank
> >>
> >>
> >> On 11/13/2017 10:34 PM, Ville Syrjala wrote:
> >>> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >>>
> >>> If the user mode would specify an aspect ratio other than 4:3 or 16:9
> >>> we now silently ignore it. Maybe a better apporoach is to return an
> >>> error? Let's try that.
> >>>
> >>> Also we must be careful that we don't try to send illegal picture
> >>> aspect in the infoframe as it's only capable of signalling none,
> >>> 4:3, and 16:9. Currently we're sending these bogus infoframes
> >>> whenever the cea mode specifies some other aspect ratio.
> >>>
> >>> Cc: Shashank Sharma <shashank.sharma@xxxxxxxxx>
> >>> Cc: Sean Paul <seanpaul@xxxxxxxxxxxx>
> >>> Cc: Jose Abreu <Jose.Abreu@xxxxxxxxxxxx>
> >>> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> >>> Cc: Emil Velikov <emil.l.velikov@xxxxxxxxx>
> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >>> ---
> >>> drivers/gpu/drm/drm_edid.c | 23 +++++++++++++++++------
> >>> 1 file changed, 17 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >>> index 00aa98f3e55d..bafb3ee4ea97 100644
> >>> --- a/drivers/gpu/drm/drm_edid.c
> >>> +++ b/drivers/gpu/drm/drm_edid.c
> >>> @@ -4786,6 +4786,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
> >>> const struct drm_display_mode *mode,
> >>> bool is_hdmi2_sink)
> >>> {
> >>> + enum hdmi_picture_aspect picture_aspect;
> >>> int err;
> >>>
> >>> if (!frame || !mode)
> >>> @@ -4828,13 +4829,23 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
> >>> * Populate picture aspect ratio from either
> >>> * user input (if specified) or from the CEA mode list.
> >>> */
> >>> - if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 ||
> >>> - mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9)
> >>> - frame->picture_aspect = mode->picture_aspect_ratio;
> >>> - else if (frame->video_code > 0)
> >>> - frame->picture_aspect = drm_get_cea_aspect_ratio(
> >>> - frame->video_code);
> >>> + picture_aspect = mode->picture_aspect_ratio;
> >>> + if (picture_aspect == HDMI_PICTURE_ASPECT_NONE)
> >>> + picture_aspect = drm_get_cea_aspect_ratio(frame->video_code);
> >> This is slightly going in the loop.
> >> - During the modeset the driver cant specify the aspect ratio
> >> information, as DRM layer lacks this support.
> >> - So we fill the VIC field, by comparing the mode with the
> >> DRM_CEA_MODES[] list. This will pick the first mode
> >> available in the list (regardless of its aspect ratio), and fill the
> >> VIC, as we don't consider aspect ratio while comparing timings.
> >> - Again, now while sending the aspect ratio, we are picking up the VIC,
> >> which may not be correct.
> >>
> >> So if we have 720x480(4:3) and 720x480(16:9) in the list, as 4:3 is
> >> first in list, we will always pick 4:3 aspect ratio.
> > Yes. The user didn't care about the aspect ratio (or rather couldn't
> > specify one) so we just pick one. Which is exactly what we've been
> > doing ever since we started sending the VIC in the infoframe.
> Correct, and we are hoping that this should be better (if not fixed)
> with the aspect ratio support
> patches + DRM cap. If the userspace doesn't set the cap, then anyways
> there is no aspect ratio
> field available, and VIC would be always 0, as this becomes a Non CEA mode.
>
> Or do you think it would be a better idea to send some VIC instead of No
> VIC, when userspace doesn't
> set the DRM cap for aspect ratio ?
Yes. That's the current behaviour.
IIRC some crappy amplifiers etc. with HDMI passthrough don't even work
correctly unless VIC is specified. Hence we do want to send it whenever
possible.
>
> - Shashank
> >> - Shashank
> >>>
> >>> + /*
> >>> + * The infoframe can't convey anything but none, 4:3
> >>> + * and 16:9, so if the user has asked for anything else
> >>> + * we can only satisfy it by specifying the right VIC.
> >>> + */
> >>> + if (picture_aspect > HDMI_PICTURE_ASPECT_16_9) {
> >>> + if (picture_aspect !=
> >>> + drm_get_cea_aspect_ratio(frame->video_code))
> >>> + return -EINVAL;
> >>> + picture_aspect = HDMI_PICTURE_ASPECT_NONE;
> >>> + }
> >>> +
> >>> + frame->picture_aspect = picture_aspect;
> >>> frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE;
> >>> frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN;
> >>>
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
