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.
Even in this case, we should go and extend the userspace (which I am
ready to do) instead of reverting the patch.
If you still think you should send this revert, I am removing my NACK.
Pls Go ahead.
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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
