On Thu, Jan 18, 2018 at 09:11:19PM +0530, ankit.k.nautiyal@xxxxxxxxx wrote: > Hi Marteen, > > Thanks for the review comments. Please find my comments in-line. > > > On Thursday 18 January 2018 03:46 PM, Maarten Lankhorst wrote: > > Op 12-01-18 om 07:21 schreef Nautiyal, Ankit K: > >> From: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx> > >> > >> A new drm client cap is required to enable user-space to advertise > >> if it supports modes with aspect-ratio. Based on this cap value, the > >> kernel will take a call on exposing the aspect ratio information in > >> modes or not. > >> > >> This patch adds the client cap for aspect-ratio. > >> > >> Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> > >> Cc: Shashank Sharma <shashank.sharma@xxxxxxxxx> > >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx> > > What's missing from the commit message is why this needs to be a flag that userspace needs to enable, > > instead of something that only counts as an informative query. > > > > I gathered from danvet that blindly exposing things breaks existing userspace, so could that information be added to the commit? > > Agreed. This information indeed brings out the real reason for the patch. > Will add the details in the commit message in the next patch-set. > > > > > Also missing IGT tests, would be nice we at least get some verification things work as expected. > > Currently this is being tested with the corresponding userspace changes > in weston compositor, but a simple IGT on the same lines of kms_3d > can be prepared. A separate test to check that the different aspect ratio modes are correctly enumerated, and filtered out when the cap is not set seems like a good idea. As for actaully testing different aspect ratio modes, maybe just intergrate that into testdisplay? -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
