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.
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 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;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
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 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;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
Regards
Shashank
On 11/3/2016 6:39 PM, Ville Syrjälä wrote:
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.
Noted for future.
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.
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. This is instead option to extend the support.
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
Again, this is not a break, we need to extend the test-setup for new flags.
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
I think we had several rounds of code reviews on this, since months. So I don't really see why are we discussing this now.
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. I dont really think how breaking / blocking Android implementation is going to help others. Also, the breaks what you are reporting are with the test tools / infrastructures, which is specific to one setup.
Honestly, thats one of the setup which uses aspect ratio in practical use cases, so I dont find a reason why not.
NACK until we get to the right reason.
My nack continues.
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 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;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
dri-devel@lists.freedesktop.org
-
Sharma, Shashank -
Ville Syrjälä