On Thu, Nov 03, 2016 at 07:04:37PM +0530, Sharma, Shashank wrote: > Regards > > Shashank > > > On 11/3/2016 6:56 PM, Ville Syrjälä wrote: > > On Thu, Nov 03, 2016 at 06:40:11PM +0530, Sharma, Shashank wrote: > >> Regards > >> > >> Shashank > >> > >> > >> On 11/3/2016 6:32 PM, Ville Syrjälä wrote: > >>> On Thu, Nov 03, 2016 at 12:47:39PM +0000, Sharma, Shashank wrote: > >>>> Hi Ville, > >>>> (-dri-devel) > >>>> I would appreciate an internal discussion before going to dri-devel. a > >>> Added back. Private discussions are pointless. > >> Noted. > >>>> What did this patch break ? > >>> Intel ddx. It will reject any mode with flags it doesn't know about. > >>> Other ddxen like -ati or -modesetting will also suffer, but potentially > >>> in a different way since they don't even check the flags and instead > >>> just directly stuff them into the randr mode flags, which will result > >>> in an out of spec mode as far randr is concerned. > >> Then its a fault of intel-ddx design. We are adding new flags to > >> indicate aspect ratios, so its obvious that the implementation should be > >> modified to accept new flags. > >> I dont see why you should revert patch for it. > > Rule 1 of kernel development: You simply don't break existing userspace > Again, we are adding something new, for which userspace should be modified. > If the userspace in written in such a way that it wont accept changes, > its not saleable design. Whether the design is good or not is besides the point. It already exists, so we have to live with it. > > Even in this case, we should go and extend the userspace (which I am > ready to do) instead of reverting the patch. We can't retroactively fix the binaries on everyone's machine. > If you still think you should send this revert, I am removing my NACK. > Pls Go ahead. The other option is to not revert and instead slap a fix on top. But that would have to be done reasonably quickly so that the thing is ready in time for 4.10. We're closing in on 4.9-rc4 now so I guess we should still have a few weeks to fix things up. Whether that's enough I don't know. If not, then we should revert. > >>>> This is exposed in the same way, as the 3D flags. > >>> Not it isn't. We have a client cap for 3D flags. > >>> > >>>> The information is already available in form of flags, all you have to do in userspace is read and print that. Its already being done in Android. > >>> Please stop relying on Android for testing upstream patches. Test with > >>> actual userspace people are using. > >> Please note that android goes through HDMI compliance certification, and > >> several commercial test infrastructures, so > >> that's one of the best infrastructure to test. And when we have common > >> branch for all of Linux flavors, we should consider all. > > You can test with Android as much as you like. That probably won't > > catch ABI breaks and whatnot though since you don't generally just plop > > a new kernel onto an existing Android device. > > > > So you really must test with normal userspace as well. That has a long > > history and so any ABI/behaviour change is actually a big deal. > > > >>>> > >>>> NACK until we get to the right reason. > >>>> > >>>> Regards > >>>> Shashank > >>>> -----Original Message----- > >>>> From: ville.syrjala@xxxxxxxxxxxxxxx [mailto:ville.syrjala@xxxxxxxxxxxxxxx] > >>>> Sent: Thursday, November 3, 2016 6:02 PM > >>>> To: dri-devel@xxxxxxxxxxxxxxxxxxxxx > >>>> Cc: Sharma, Shashank <shashank.sharma@xxxxxxxxx>; Lin; Jia, Lin A <lin.a.jia@xxxxxxxxx>; Sharma, Akashdeep <akashdeep.sharma@xxxxxxxxx>; Jim Bride <jim.bride@xxxxxxxxxxxxxxx>; Jose Abreu <Jose.Abreu@xxxxxxxxxxxx>; Daniel Vetter <daniel.vetter@xxxxxxxx>; Emil Velikov <emil.l.velikov@xxxxxxxxx> > >>>> Subject: [PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer" > >>>> > >>>> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > >>>> > >>>> This reverts commit a68362fe3e84fcbedd49939aa200519aa5410135. > >>>> > >>>> Adding new mode flags willy nilly breaks existing userspace. We need to coordinate this better, potentially with a new client cap that only exposes the aspect ratio flags when userspace is prepared for them (similar to what we do with stereo 3D modes). > >>>> > >>>> Cc: Shashank Sharma <shashank.sharma@xxxxxxxxx> > >>>> Cc: Lin, Jia <lin.a.jia@xxxxxxxxx> > >>>> Cc: Akashdeep Sharma <akashdeep.sharma@xxxxxxxxx> > >>>> Cc: Jim Bride <jim.bride@xxxxxxxxxxxxxxx> > >>>> Cc: Jose Abreu <Jose.Abreu@xxxxxxxxxxxx> > >>>> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > >>>> Cc: Emil Velikov <emil.l.velikov@xxxxxxxxx> > >>>> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > >>>> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > >>>> --- > >>>> drivers/gpu/drm/drm_modes.c | 12 ------------ include/uapi/drm/drm_mode.h | 6 ------ > >>>> 2 files changed, 18 deletions(-) > >>>> > >>>> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index f64ac86deb84..725faa6409aa 100644 > >>>> --- a/drivers/gpu/drm/drm_modes.c > >>>> +++ b/drivers/gpu/drm/drm_modes.c > >>>> @@ -1513,12 +1513,6 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out, > >>>> case HDMI_PICTURE_ASPECT_16_9: > >>>> out->flags |= DRM_MODE_FLAG_PIC_AR_16_9; > >>>> break; > >>>> - case HDMI_PICTURE_ASPECT_64_27: > >>>> - out->flags |= DRM_MODE_FLAG_PIC_AR_64_27; > >>>> - break; > >>>> - case DRM_MODE_PICTURE_ASPECT_256_135: > >>>> - out->flags |= DRM_MODE_FLAG_PIC_AR_256_135; > >>>> - break; > >>>> case HDMI_PICTURE_ASPECT_RESERVED: > >>>> default: > >>>> out->flags |= DRM_MODE_FLAG_PIC_AR_NONE; @@ -1580,12 +1574,6 @@ int drm_mode_convert_umode(struct drm_display_mode *out, > >>>> case DRM_MODE_FLAG_PIC_AR_16_9: > >>>> out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_16_9; > >>>> break; > >>>> - case DRM_MODE_FLAG_PIC_AR_64_27: > >>>> - out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_64_27; > >>>> - break; > >>>> - case DRM_MODE_FLAG_PIC_AR_256_135: > >>>> - out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_256_135; > >>>> - break; > >>>> default: > >>>> out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; > >>>> break; > >>>> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 084b50a02dc5..5c142b1387ac 100644 > >>>> --- a/include/uapi/drm/drm_mode.h > >>>> +++ b/include/uapi/drm/drm_mode.h > >>>> @@ -81,8 +81,6 @@ extern "C" { > >>>> #define DRM_MODE_PICTURE_ASPECT_NONE 0 > >>>> #define DRM_MODE_PICTURE_ASPECT_4_3 1 > >>>> #define DRM_MODE_PICTURE_ASPECT_16_9 2 > >>>> -#define DRM_MODE_PICTURE_ASPECT_64_27 3 > >>>> -#define DRM_MODE_PICTURE_ASPECT_256_135 4 > >>>> > >>>> /* Aspect ratio flag bitmask (4 bits 22:19) */ > >>>> #define DRM_MODE_FLAG_PIC_AR_MASK (0x0F<<19) > >>>> @@ -92,10 +90,6 @@ extern "C" { > >>>> (DRM_MODE_PICTURE_ASPECT_4_3<<19) > >>>> #define DRM_MODE_FLAG_PIC_AR_16_9 \ > >>>> (DRM_MODE_PICTURE_ASPECT_16_9<<19) > >>>> -#define DRM_MODE_FLAG_PIC_AR_64_27 \ > >>>> - (DRM_MODE_PICTURE_ASPECT_64_27<<19) > >>>> -#define DRM_MODE_FLAG_PIC_AR_256_135 \ > >>>> - (DRM_MODE_PICTURE_ASPECT_256_135<<19) > >>>> > >>>> /* DPMS flags */ > >>>> /* bit compatible with the xorg definitions. */ > >>>> -- > >>>> 2.7.4 > >>>> -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
