From: Ville Syrjälä ville.syrjala@linux.intel.com
Ilia pointed out some oddball crap in the i915 aspect ratio handling. While looking at that I noticed a bunch of fail in the core as well. This series aims to fix it all.
Cc: Ilia Mirkin imirkin@alum.mit.edu
Ville Syrjälä (5): drm: Do not use bitwise OR to set picure_aspect_ratio drm: Do not accept garbage mode aspect ratio flags drm: WARN on illegal aspect ratio when converting a mode to umode drm/i915: Do not override mode's aspect ratio with the prop value NONE drm/i915: Drop redundant aspec ratio prop value initialization
drivers/gpu/drm/drm_modes.c | 17 +++++++++++------ drivers/gpu/drm/i915/display/intel_hdmi.c | 5 ++--- drivers/gpu/drm/i915/display/intel_sdvo.c | 4 +--- 3 files changed, 14 insertions(+), 12 deletions(-)
From: Ville Syrjälä ville.syrjala@linux.intel.com
enum hdmi_picture_aspect is not a bitmask, so don't use bitwise OR to populate it.
Cc: Ilia Mirkin imirkin@alum.mit.edu Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_modes.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 57e6408288c8..53acc6756ee0 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1966,16 +1966,16 @@ int drm_mode_convert_umode(struct drm_device *dev,
switch (in->flags & DRM_MODE_FLAG_PIC_AR_MASK) { case DRM_MODE_FLAG_PIC_AR_4_3: - out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_4_3; + out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3; break; case DRM_MODE_FLAG_PIC_AR_16_9: - out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_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; + 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; + out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135; break; default: out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
From: Ville Syrjälä ville.syrjala@linux.intel.com
Don't let userspace feed us any old garbage in the mode aspect ratio flags.
Cc: Ilia Mirkin imirkin@alum.mit.edu Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_modes.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 53acc6756ee0..847048dee048 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1977,9 +1977,11 @@ int drm_mode_convert_umode(struct drm_device *dev, case DRM_MODE_FLAG_PIC_AR_256_135: out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135; break; - default: + case DRM_MODE_FLAG_PIC_AR_NONE: out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; break; + default: + return -EINVAL; }
out->status = drm_mode_validate_driver(dev, out);
From: Ville Syrjälä ville.syrjala@linux.intel.com
WARN if the incoming drm_display_mode has an illegal aspect ratio when converting it to a user mode. This should never happen unless the driver made a mistake and put an invalid value into the aspect ratio.
Cc: Ilia Mirkin imirkin@alum.mit.edu Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_modes.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 847048dee048..be2ccd8eccfd 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1906,8 +1906,11 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out, case HDMI_PICTURE_ASPECT_256_135: out->flags |= DRM_MODE_FLAG_PIC_AR_256_135; break; - case HDMI_PICTURE_ASPECT_RESERVED: default: + WARN(1, "Invalid aspect ratio (0%x) on mode\n", + in->picture_aspect_ratio); + /* fall through */ + case HDMI_PICTURE_ASPECT_NONE: out->flags |= DRM_MODE_FLAG_PIC_AR_NONE; break; }
From: Ville Syrjälä ville.syrjala@linux.intel.com
HDMI_PICTURE_ASPECT_NONE means "Automatic" so when the user has that selected we should keep whatever aspect ratio the mode already has.
Also no point in checking for connector->is_hdmi in the SDVO code since we only attach the property to HDMI connectors.
Cc: Ilia Mirkin imirkin@alum.mit.edu Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/display/intel_hdmi.c | 5 +++-- drivers/gpu/drm/i915/display/intel_sdvo.c | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index 0ebec69bbbfc..6a4650b44ac6 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2394,8 +2394,9 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder, return -EINVAL; }
- /* Set user selected PAR to incoming mode's member */ - adjusted_mode->picture_aspect_ratio = conn_state->picture_aspect_ratio; + if (conn_state->picture_aspect_ratio) + adjusted_mode->picture_aspect_ratio = + conn_state->picture_aspect_ratio;
pipe_config->lane_count = 4;
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c index ceda03e5a3d4..5cb619613157 100644 --- a/drivers/gpu/drm/i915/display/intel_sdvo.c +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c @@ -1321,9 +1321,9 @@ static int intel_sdvo_compute_config(struct intel_encoder *encoder, if (IS_TV(intel_sdvo_connector)) i9xx_adjust_sdvo_tv_clock(pipe_config);
- /* Set user selected PAR to incoming mode's member */ - if (intel_sdvo_connector->is_hdmi) - adjusted_mode->picture_aspect_ratio = conn_state->picture_aspect_ratio; + if (conn_state->picture_aspect_ratio) + adjusted_mode->picture_aspect_ratio = + conn_state->picture_aspect_ratio;
if (!intel_sdvo_compute_avi_infoframe(intel_sdvo, pipe_config, conn_state)) {
-----Original Message----- From: dri-devel dri-devel-bounces@lists.freedesktop.org On Behalf Of Ville Syrjala Sent: Thursday, June 20, 2019 7:57 PM To: dri-devel@lists.freedesktop.org Cc: intel-gfx@lists.freedesktop.org Subject: [PATCH 4/5] drm/i915: Do not override mode's aspect ratio with the prop value NONE
From: Ville Syrjälä ville.syrjala@linux.intel.com
HDMI_PICTURE_ASPECT_NONE means "Automatic" so when the user has that selected we should keep whatever aspect ratio the mode already has.
Also no point in checking for connector->is_hdmi in the SDVO code since we only attach the property to HDMI connectors.
Looks good to me. Reviewed-by: Uma Shankar uma.shankar@intel.com
Cc: Ilia Mirkin imirkin@alum.mit.edu Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/i915/display/intel_hdmi.c | 5 +++-- drivers/gpu/drm/i915/display/intel_sdvo.c | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index 0ebec69bbbfc..6a4650b44ac6 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2394,8 +2394,9 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder, return -EINVAL; }
- /* Set user selected PAR to incoming mode's member */
- adjusted_mode->picture_aspect_ratio = conn_state->picture_aspect_ratio;
if (conn_state->picture_aspect_ratio)
adjusted_mode->picture_aspect_ratio =conn_state->picture_aspect_ratio;pipe_config->lane_count = 4;
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c index ceda03e5a3d4..5cb619613157 100644 --- a/drivers/gpu/drm/i915/display/intel_sdvo.c +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c @@ -1321,9 +1321,9 @@ static int intel_sdvo_compute_config(struct intel_encoder *encoder, if (IS_TV(intel_sdvo_connector)) i9xx_adjust_sdvo_tv_clock(pipe_config);
- /* Set user selected PAR to incoming mode's member */
- if (intel_sdvo_connector->is_hdmi)
adjusted_mode->picture_aspect_ratio = conn_state-picture_aspect_ratio;
if (conn_state->picture_aspect_ratio)
adjusted_mode->picture_aspect_ratio =conn_state->picture_aspect_ratio;if (!intel_sdvo_compute_avi_infoframe(intel_sdvo, pipe_config, conn_state)) {
-- 2.21.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
From: Ville Syrjälä ville.syrjala@linux.intel.com
HDMI_PICTURE_ASPECT_NONE is zero and the connector state is kzalloc()'d so no need to initialize conn_state->picture_aspect_ratio with it.
Cc: Ilia Mirkin imirkin@alum.mit.edu Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/display/intel_hdmi.c | 1 - drivers/gpu/drm/i915/display/intel_sdvo.c | 1 - 2 files changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index 6a4650b44ac6..f95f3db5ecb8 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2809,7 +2809,6 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c intel_attach_colorspace_property(connector);
drm_connector_attach_content_type_property(connector); - connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) drm_object_attach_property(&connector->base, diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c index 5cb619613157..c471fcce59f8 100644 --- a/drivers/gpu/drm/i915/display/intel_sdvo.c +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c @@ -2624,7 +2624,6 @@ intel_sdvo_add_hdmi_properties(struct intel_sdvo *intel_sdvo, intel_attach_broadcast_rgb_property(&connector->base.base); } intel_attach_aspect_ratio_property(&connector->base.base); - connector->base.base.state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; }
static struct intel_sdvo_connector *intel_sdvo_connector_alloc(void)
-----Original Message----- From: Intel-gfx intel-gfx-bounces@lists.freedesktop.org On Behalf Of Ville Syrjala Sent: Thursday, June 20, 2019 7:57 PM To: dri-devel@lists.freedesktop.org Cc: intel-gfx@lists.freedesktop.org; Ilia Mirkin imirkin@alum.mit.edu Subject: [Intel-gfx] [PATCH 5/5] drm/i915: Drop redundant aspec ratio prop value initialization
From: Ville Syrjälä ville.syrjala@linux.intel.com
HDMI_PICTURE_ASPECT_NONE is zero and the connector state is kzalloc()'d so no need to initialize conn_state->picture_aspect_ratio with it.
Looks good to me. Reviewed-by: Uma Shankar uma.shankar@intel.com
Cc: Ilia Mirkin imirkin@alum.mit.edu Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/i915/display/intel_hdmi.c | 1 - drivers/gpu/drm/i915/display/intel_sdvo.c | 1 - 2 files changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index 6a4650b44ac6..f95f3db5ecb8 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2809,7 +2809,6 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c intel_attach_colorspace_property(connector);
drm_connector_attach_content_type_property(connector);
connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) drm_object_attach_property(&connector->base,
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c index 5cb619613157..c471fcce59f8 100644 --- a/drivers/gpu/drm/i915/display/intel_sdvo.c +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c @@ -2624,7 +2624,6 @@ intel_sdvo_add_hdmi_properties(struct intel_sdvo *intel_sdvo, intel_attach_broadcast_rgb_property(&connector->base.base); } intel_attach_aspect_ratio_property(&connector->base.base);
- connector->base.base.state->picture_aspect_ratio =
HDMI_PICTURE_ASPECT_NONE; }
static struct intel_sdvo_connector *intel_sdvo_connector_alloc(void)
2.21.0
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Jun 20, 2019 at 10:26 AM Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Ilia pointed out some oddball crap in the i915 aspect ratio handling. While looking at that I noticed a bunch of fail in the core as well. This series aims to fix it all.
Cc: Ilia Mirkin imirkin@alum.mit.edu
Ville Syrjälä (5): drm: Do not use bitwise OR to set picure_aspect_ratio drm: Do not accept garbage mode aspect ratio flags drm: WARN on illegal aspect ratio when converting a mode to umode
Patches 1-3: Reviewed-by: Alex Deucher alexander.deucher@amd.com
drm/i915: Do not override mode's aspect ratio with the prop value NONE drm/i915: Drop redundant aspec ratio prop value initialization
drivers/gpu/drm/drm_modes.c | 17 +++++++++++------ drivers/gpu/drm/i915/display/intel_hdmi.c | 5 ++--- drivers/gpu/drm/i915/display/intel_sdvo.c | 4 +--- 3 files changed, 14 insertions(+), 12 deletions(-)
-- 2.21.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, Jun 21, 2019 at 07:28:30PM -0400, Alex Deucher wrote:
On Thu, Jun 20, 2019 at 10:26 AM Ville Syrjala ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Ilia pointed out some oddball crap in the i915 aspect ratio handling. While looking at that I noticed a bunch of fail in the core as well. This series aims to fix it all.
Cc: Ilia Mirkin imirkin@alum.mit.edu
Ville Syrjälä (5): drm: Do not use bitwise OR to set picure_aspect_ratio drm: Do not accept garbage mode aspect ratio flags drm: WARN on illegal aspect ratio when converting a mode to umode
Patches 1-3: Reviewed-by: Alex Deucher alexander.deucher@amd.com
Thanks. 1-3 pushed to drm-misc-next.
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Shankar, Uma -
Ville Syrjala -
Ville Syrjälä