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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Nov 03, 2016 at 03:04:04PM +0200, Ville Syrjälä wrote:
> On Thu, Nov 03, 2016 at 03:02:53PM +0200, 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.
> 
> Now added for reals.
> 
> > 
> > > 
> > > 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.

BTW outside of the userspace issues there were some issues with the way
the mode matching was being done. Before I realized the userspace breakage
I had cooked up a few additonal patch to adjust the mode matching a bit.
I pushed those to [1] in case anyone wants to take a look, but obviously
they need to be put on hold a bit until we sort out the userspace part.

[1] git://github.com/vsyrjala/linux.git cea_f_vics

> > 
> > > 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.
> > 
> > >  
> > > NACK until we get to the right reason. 
> > > 
> > > Regards
> > > Shashank
> > > -----Original Message-----
> > > From: ville.syrjala@linux.intel.com [mailto:ville.syrjala@linux.intel.com] 
> > > Sent: Thursday, November 3, 2016 6:02 PM
> > > To: dri-devel@lists.freedesktop.org
> > > Cc: Sharma, Shashank <shashank.sharma@intel.com>; Lin; Jia, Lin A <lin.a.jia@intel.com>; Sharma, Akashdeep <akashdeep.sharma@intel.com>; Jim Bride <jim.bride@linux.intel.com>; Jose Abreu <Jose.Abreu@synopsys.com>; Daniel Vetter <daniel.vetter@ffwll.ch>; Emil Velikov <emil.l.velikov@gmail.com>
> > > Subject: [PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
> > > 
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > 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@intel.com>
> > > Cc: Lin, Jia <lin.a.jia@intel.com>
> > > Cc: Akashdeep Sharma <akashdeep.sharma@intel.com>
> > > Cc: Jim Bride <jim.bride@linux.intel.com>
> > > Cc: Jose Abreu <Jose.Abreu@synopsys.com>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Cc: Emil Velikov <emil.l.velikov@gmail.com>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  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
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel