On Wed, Jul 05, 2017 at 03:49:51PM +0530, Sharma, Shashank wrote:
Regards
Shashank
On 7/5/2017 3:46 PM, Ville Syrjälä wrote:
On Wed, Jul 05, 2017 at 08:48:40AM +0530, Sharma, Shashank wrote:
Regards
Shashank
On 7/4/2017 9:26 PM, Ville Syrjälä wrote:
On Tue, Jul 04, 2017 at 07:41:51PM +0530, Shashank Sharma wrote:
YCBCR420 modes are supported only on HDMI 2.0 capable sources. This patch adds a drm helper to validate YCBCR420-only mode on a particular connector. This function will help pruning the YCBCR420-only modes from the connector's modelist.
V5: Introduced the patch in series.
Cc: Ville Syrjala ville.syrjala@linux.intel.com Signed-off-by: Shashank Sharma shashank.sharma@intel.com
drivers/gpu/drm/drm_edid.c | 3 ++- drivers/gpu/drm/drm_modes.c | 28 ++++++++++++++++++++++++++++ drivers/gpu/drm/drm_probe_helper.c | 4 ++++ include/drm/drm_edid.h | 1 + include/drm/drm_modes.h | 5 +++++ 5 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index b879662..ad26c5e 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2947,10 +2947,11 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) } EXPORT_SYMBOL(drm_match_cea_mode);
-static bool drm_valid_cea_vic(u8 vic) +bool drm_valid_cea_vic(u8 vic) { return vic > 0 && vic < ARRAY_SIZE(edid_cea_modes); } +EXPORT_SYMBOL(drm_valid_cea_vic);
/** * drm_get_cea_aspect_ratio - get the picture aspect ratio corresponding to diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index f2493b9..3b53c8e3 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1083,6 +1083,34 @@ drm_mode_validate_size(const struct drm_display_mode *mode, } EXPORT_SYMBOL(drm_mode_validate_size);
+/**
- drm_mode_ycbcr420_only - add 'ycbcr420-only' modes only when allowed
- @mode: mode to check
- @connector: drm connector under action
- This function is a helper which can be used to filter out any YCBCR420
- only mode, when the source doesn't support it.
- Returns:
- The mode status
- */
+enum drm_mode_status +drm_mode_validate_ycbcr420(const struct drm_display_mode *mode,
struct drm_connector *connector)+{
- u8 vic = drm_match_cea_mode(mode);
- enum drm_mode_status status = MODE_OK;
- struct drm_hdmi_info *hdmi = &connector->display_info.hdmi;
- if (drm_valid_cea_vic(vic) && test_bit(vic, hdmi->y420_vdb_modes)) {
The drm_valid_cea_vic() check seems redundant to me. Why do you think we need it?
drm_match_cea_mode() returns 0 in case if a vic is not found. if we take that to test_bit() it will check bit 0 and give wrong results.
Why would bit 0 be set? That would sound like a bug in the mode parsing.
I know, and it wont be, but do we want to take the wrong VIC as input in first place ? Many detailed modes, and non-cea modes will return 0 for VIC,
All non-cea modes will return 0.
why should we bother checking them in map at the first place ?
Checking the vic against some range is pointless work. Just checking the bit blindly is the most optimal way.
- Shashank
So first we have to check valid vic, this is also an additional check for YCBCR420 mode as they must have a valid vic
Also I don't think this will compile since we don't have y420_vdb_modes[] yet.
Ah, now I recall the reason I wanted to give you, when you asked me to move this patch, before we add ycbcr420 modes. So now this has to go after patch 5 right ? I would re-sequence accordingly.
- Shashank
if (!connector->ycbcr_420_allowed)status = MODE_NO_420;- }
- return status;
+} +EXPORT_SYMBOL(drm_mode_validate_ycbcr420);
#define MODE_STATUS(status) [MODE_ ## status + 3] = #status
static const char * const drm_mode_status_names[] = {
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 00e6832..904966c 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -528,6 +528,10 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, if (mode->status == MODE_OK) mode->status = drm_mode_validate_pipeline(mode, connector);
if (mode->status == MODE_OK)mode->status = drm_mode_validate_ycbcr420(mode,connector);}
prune:
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 89c0062..b55b2a7 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -477,4 +477,5 @@ void drm_edid_get_monitor_name(struct edid *edid, char *name, struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev, int hsize, int vsize, int fresh, bool rb); +bool drm_valid_cea_vic(u8 vic); #endif /* __DRM_EDID_H__ */ diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 94ac771..f8a1268 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -80,6 +80,7 @@ struct videomode; * @MODE_ONE_SIZE: only one resolution is supported * @MODE_NO_REDUCED: monitor doesn't accept reduced blanking * @MODE_NO_STEREO: stereo modes not supported
- @MODE_NO_420: ycbcr 420 modes not supported
- @MODE_STALE: mode has become stale
- @MODE_BAD: unspecified reason
- @MODE_ERROR: error condition
@@ -124,6 +125,7 @@ enum drm_mode_status { MODE_ONE_SIZE, MODE_NO_REDUCED, MODE_NO_STEREO,
- MODE_NO_420, MODE_STALE = -3, MODE_BAD = -2, MODE_ERROR = -1
@@ -496,6 +498,9 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1, enum drm_mode_status drm_mode_validate_basic(const struct drm_display_mode *mode); enum drm_mode_status drm_mode_validate_size(const struct drm_display_mode *mode, int maxX, int maxY); +enum drm_mode_status +drm_mode_validate_ycbcr420(const struct drm_display_mode *mode,
struct drm_connector *connector);-- 2.7.4