[PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"

[Sharma, Shashank]

Regards
Shashank
On 11/15/2016 7:48 PM, Ville Syrjälä wrote:
> On Tue, Nov 15, 2016 at 01:48:04PM +0000, Jose Abreu wrote:
>> Hi,
>>
>>
>>
>> On 15-11-2016 10:52, Daniel Vetter wrote:
>>> On Tue, Nov 15, 2016 at 03:36:02PM +0530, Sharma, Shashank wrote:
>>>> On 11/15/2016 3:30 PM, Daniel Vetter wrote:
>>>>> On Tue, Nov 15, 2016 at 02:30:47PM +0530, Sharma, Shashank wrote:
>>>>>> On 11/15/2016 2:21 PM, Daniel Vetter wrote:
>>>>>>> On Mon, Nov 14, 2016 at 10:26:08PM +0530, Sharma, Shashank wrote:
>>>>>>>> In any case, I guess addition of a cap for aspect ratio should fix the
>>>>>>>> current objections for this implementation.
>>>>>>>>
>>>>>>>> And I will keep it 0 by default, so that no aspect ratio information is
>>>>>>>> added until userspace sets the cap to 1 on its own.
>>>>>>> Note that cap = needs new userspace.
>>>>>>> -Daniel
>>>>>> I guess you mean a new libdrm, so yes, I will add this new cap in libdrm.
>>>>>> Is that what you mean ?
>>>>> Full stack solution, including enabling in an Xorg driver (or somewhere
>>>>> else, we also have drm_hwcomposer as an option).
>>>>>
>>>>> And because that's probably going to take forever I'm leaning towards
>>>>> revert again. Ville?
>>>> Not really, with a kernel cap implementation, the code will be as it was
>>>> before this patch series ( as good as revert )
>>>> And then when we want to enable it, we can add the corresponding Xserver
>>>> patch.
>>>>
>>>> I guess the current problem is if is breaks something in some userspace, but
>>>> if I am loading the flags only when the cap is set, we should be good.
>>> This is not how new uabi gets merged, see:
>>>
>>> https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements
>>>
>>> Userspace must be ready, or it will not land. The entire point of my
>>> suggestion to just extend the display modes was to avoid the need for
>>> userspace, but since existing userspace tries to be too clever already,
>>> that didn't work out.
>>> Thanks, Daniel
>> @Ville
>>
>> I gave my tested by to these patches because I was facing the
>> same scenario as Shashank: HDMI compliance. I believe we need to
>> find a better way to handle this instead of just reverting. The
>> HDMI spec is evolving so we also need to evolve. Of course that
>> if this is breaking user space then we can revert and wait until
>> we figure the best way to implement this.
>>
>> Anyway, this is a wild guess but I think the reason you are
>> loosing modes may be because of
>> drm_mode_equal_no_clocks_no_stereo() which is always expecting a
>> aspect ratio flag to be defined. If there isn't one defined then
>> it will unconditionally return false, even if the modes are
>> equal.
> I am well aware why we're losing modes. One reason is the mode equal
> checks in the kernel as you suspect, the other is simply userspace
> rejecting everything with the aspect ratio defined.
>
>> Can you please test this patch and see if it fixes on your
>> side? WARNING: Not compile tested
> I already did something like that earlier, except I rewrote the
> entire messy mode matching code to use a cleaner flag based approach:
>
> git://github.com/vsyrjala/linux.git cea_f_vics
>
> But that doesn't solve the userspace ABI issue, and there are still a
> lot of unanswered questions like:
>
> - Should we define aspect ratios for modes not directly coming from
>    edid_cea_modes[]?
>
>    I beleive the answer must be "yes" at least in the cases where the
>    EDID lists the VIC even if we then skip adding the mode due to
>    already having it on the list via from another source. Perhaps we
>    want the aspect ratio even if the VIC wasn't directly specified?
Wouldn't this break the whole concept of VIC, which is supposed to point 
to a unique mode ?
> - Should we or should we not specify a VIC in the AVI infoframe
>    when the userspace doesn't support aspect ratio flags?
This is a requirement of HDMI compliance tests, if userspace 
supports/handles aspect ratio, it should set cap, and it will be set in 
kernel flags
and we should load that in AV IF.  Are you planning to suggest a better 
way, in which this can be done ?
>    I would guess the answer is again "yes", and we should just pick the
>    preferred aspect ratio for the mode. At least that's closer to what
>    we've been doing up to now (except we just picked the first VIC, so
>    we might have picked the non-preferred aspect ratio actually). At
>    least having the VIC in the infoframe would seem like a safer option
>    than not having it, in case there's some cheap ass hardware that
>    can't do anything sensible without being hand fed a VIC.
>
> - How we should handle the aspect ratio in mode sorting?
>
>    I think we want some sensible sorting order for these. Preferred
>    aspect ratio first would seem like the obvious choice. I do realize
>    that we don't sort based on 3D stereo flags either, but I thinking we
>    probably should.
What is the need of this sorting ? None of the standards define/suggest 
this. AFAIK, the requirement is that only the preferred mode should be 
notified
to the userspace, if a system has to take a call, it can create its 
policy, if a user has to take a call, it can pick any from the list.