[PATCH v2 3/4] drm/i915: Add "panel orientation" property to the panel connector
Hans de Goede
hdegoede at redhat.com
Mon Oct 23 06:57:15 UTC 2017
Hi,
On 02-10-17 10:06, Daniel Vetter wrote:
> On Sun, Oct 01, 2017 at 05:33:16PM +0200, Hans de Goede wrote:
>> Ideally we could use the VBT for this, that would be simple, in
>> intel_dsi_init() check dev_priv->vbt.dsi.config->rotation, set
>> connector->display_info.panel_orientation accordingly and call
>> drm_connector_init_panel_orientation_property(), done.
>>
>> Unfortunately vbt.dsi.config->rotation is always 0 even on tablets
>> with an upside down LCD and where the GOP is properly rotating the
>> EFI fb in hardware.
>>
>> So instead we end up reading the rotation from the primary plane,
>> which is a bit more complicated.
>>
>> The code to read back the rotation from the hardware is based on an
>> earlier attempt to fix fdo#94894 by Ville Syrjala.
>>
>> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=94894
>> Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
>> Suggested-by: Ville Syrjala <ville.syrjala at linux.intel.com>
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>
> This patch looks a bit like it tries to maximize layering violations:
>
> - store panel state in plane_state
> - digg a hole from global code to set up panel information
>
> Can't we do this in a better way? I'm thinking of something mimicking the
> fixed mode readout, which is also not driven in a backwards way like this.
> I.e. a small helper in intel_panel.c which reads out the rotation of the
> primary panel and hopes for the best.
Ok, I've reworked this in a way which will hopefully be much more to
your liking. This is now pretty much all isolated to intel_dsi.c
> Plus ofc taking the quirk list into account.
>
> Also, how exactly does the GOP figure out we need to rotate?
I wish I knew, the logical answer would be the rotation field of
struct mipi_config (which is part of the VBT) but at least no the
tablet with upside-down LCD panel I've using that field is not the
answer, hence the code to read it back from the hw at boot.
Regards,
Hans
>> ---
>> Changes in v2:
>> -Rebased on 4.14-rc1
>> -Readback primary plane rotation from the hardware and use that to
>> set the panel orientation
>> -This (readback) fixes fdo#94894, add Fixes: tag
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 3 +++
>> drivers/gpu/drm/i915/intel_display.c | 39 +++++++++++++++++++++++++++++++++++-
>> drivers/gpu/drm/i915/intel_drv.h | 5 +++++
>> drivers/gpu/drm/i915/intel_panel.c | 33 ++++++++++++++++++++++++++++++
>> 4 files changed, 79 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 18d9da53282b..c4c8590972b4 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2151,6 +2151,8 @@ struct intel_cdclk_state {
>> unsigned int cdclk, vco, ref;
>> };
>>
>> +struct intel_panel;
>> +
>> struct drm_i915_private {
>> struct drm_device drm;
>>
>> @@ -2200,6 +2202,7 @@ struct drm_i915_private {
>> struct i915_gem_context *kernel_context;
>> struct intel_engine_cs *engine[I915_NUM_ENGINES];
>> struct i915_vma *semaphore;
>> + struct intel_panel *panel;
>>
>> struct drm_dma_handle *status_page_dmah;
>> struct resource mch_res;
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 00cd17c76fdc..fbd61f92b60d 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -2891,6 +2891,9 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>> return;
>> }
>>
>> + intel_panel_set_orientation_from_crtc(dev_priv->panel, intel_crtc,
>> + plane_config->panel_orientation);
>> +
>> plane_state->src_x = 0;
>> plane_state->src_y = 0;
>> plane_state->src_w = fb->width << 16;
>> @@ -7523,6 +7526,10 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
>> plane_config->tiling = I915_TILING_X;
>> fb->modifier = I915_FORMAT_MOD_X_TILED;
>> }
>> +
>> + if (val & DISPPLANE_ROTATE_180)
>> + plane_config->panel_orientation =
>> + DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
>> }
>>
>> pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
>> @@ -8576,6 +8583,24 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
>> goto error;
>> }
>>
>> + /* The rotation is used to *correct* for the panel orientation */
>> + switch (val & PLANE_CTL_ROTATE_MASK) {
>> + case PLANE_CTL_ROTATE_0:
>> + break;
>> + case PLANE_CTL_ROTATE_90:
>> + plane_config->panel_orientation =
>> + DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
>> + break;
>> + case PLANE_CTL_ROTATE_180:
>> + plane_config->panel_orientation =
>> + DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
>> + break;
>> + case PLANE_CTL_ROTATE_270:
>> + plane_config->panel_orientation =
>> + DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
>> + break;
>> + }
>> +
>> base = I915_READ(PLANE_SURF(pipe, 0)) & 0xfffff000;
>> plane_config->base = base;
>>
>> @@ -8661,6 +8686,10 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc,
>> plane_config->tiling = I915_TILING_X;
>> fb->modifier = I915_FORMAT_MOD_X_TILED;
>> }
>> +
>> + if (val & DISPPLANE_ROTATE_180)
>> + plane_config->panel_orientation =
>> + DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
>> }
>>
>> pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
>> @@ -14578,7 +14607,9 @@ int intel_modeset_init(struct drm_device *dev)
>> drm_modeset_unlock_all(dev);
>>
>> for_each_intel_crtc(dev, crtc) {
>> - struct intel_initial_plane_config plane_config = {};
>> + struct intel_initial_plane_config plane_config = {
>> + .panel_orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL,
>> + };
>>
>> if (!crtc->active)
>> continue;
>> @@ -14600,6 +14631,12 @@ int intel_modeset_init(struct drm_device *dev)
>> intel_find_initial_plane_obj(crtc, &plane_config);
>> }
>>
>> + /*
>> + * Init panel orientation prop now that intel_find_initial_plane_obj()
>> + * has had a chance to set panel orientation.
>> + */
>> + intel_panel_init_orientation_prop(dev_priv->panel);
>> +
>> /*
>> * Make sure hardware watermarks really match the state we read out.
>> * Note that we need to do this after reconstructing the BIOS fb's
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index fa47285918f4..50fa0eeca655 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -447,6 +447,7 @@ struct intel_initial_plane_config {
>> unsigned int tiling;
>> int size;
>> u32 base;
>> + int panel_orientation; /* DRM_MODE_PANEL_ORIENTATION_* */
>> };
>>
>> #define SKL_MIN_SRC_W 8
>> @@ -1703,6 +1704,10 @@ extern struct drm_display_mode *intel_find_panel_downclock(
>> struct drm_i915_private *dev_priv,
>> struct drm_display_mode *fixed_mode,
>> struct drm_connector *connector);
>> +void intel_panel_set_orientation_from_crtc(struct intel_panel *panel,
>> + struct intel_crtc *intel_crtc,
>> + int orientation);
>> +void intel_panel_init_orientation_prop(struct intel_panel *panel);
>>
>> #if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>> int intel_backlight_device_register(struct intel_connector *connector);
>> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
>> index 0f7942a5dccf..fa7dfb9ac5f0 100644
>> --- a/drivers/gpu/drm/i915/intel_panel.c
>> +++ b/drivers/gpu/drm/i915/intel_panel.c
>> @@ -1925,11 +1925,44 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
>> }
>> }
>>
>> +void intel_panel_set_orientation_from_crtc(struct intel_panel *panel,
>> + struct intel_crtc *intel_crtc,
>> + int orientation)
>> +{
>> + struct intel_connector *panel_conn;
>> +
>> + if (!panel)
>> + return;
>> +
>> + panel_conn = container_of(panel, struct intel_connector, panel);
>> + if (panel_conn->base.state->crtc == &intel_crtc->base)
>> + panel_conn->base.display_info.panel_orientation = orientation;
>> +}
>> +
>> +void intel_panel_init_orientation_prop(struct intel_panel *panel)
>> +{
>> + struct intel_connector *panel_conn;
>> +
>> + if (!panel || !panel->fixed_mode)
>> + return;
>> +
>> + panel_conn = container_of(panel, struct intel_connector, panel);
>> + drm_connector_init_panel_orientation_property(&panel_conn->base,
>> + panel->fixed_mode->hdisplay, panel->fixed_mode->vdisplay);
>> +}
>> +
>> int intel_panel_init(struct intel_panel *panel,
>> struct drm_display_mode *fixed_mode,
>> struct drm_display_mode *alt_fixed_mode,
>> struct drm_display_mode *downclock_mode)
>> {
>> + struct intel_connector *intel_connector =
>> + container_of(panel, struct intel_connector, panel);
>> + struct drm_i915_private *dev_priv = to_i915(intel_connector->base.dev);
>> +
>> + if (!dev_priv->panel)
>> + dev_priv->panel = panel;
>> +
>> intel_panel_init_backlight_funcs(panel);
>>
>> panel->fixed_mode = fixed_mode;
>> --
>> 2.14.2
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
More information about the dri-devel
mailing list