On Wed, Apr 17, 2019 at 07:56:20PM +0200, Noralf Trønnes wrote:
>
>
> Den 16.04.2019 10.38, skrev Daniel Vetter:
> > On Sun, Apr 07, 2019 at 06:52:39PM +0200, Noralf Trønnes wrote:
> >> Move the modeset commit code to drm_client_modeset.
> >> No changes except exporting API.
> >>
> >> v2: Move to drm_client_modeset.c instead of drm_client.c
> >>
> >> Signed-off-by: Noralf Trønnes <nor...@tronnes.org>
> >> ---
> >> drivers/gpu/drm/drm_client_modeset.c | 287 +++++++++++++++++++++++++++
>
> <snip>
>
> >> -/**
> >> - * drm_client_panel_rotation() - Check panel orientation
> >> - * @modeset: DRM modeset
> >> - * @rotation: Returned rotation value
> >> - *
> >> - * This function checks if the primary plane in @modeset can hw rotate to
> >> match
> >> - * the panel orientation on its connector.
> >> - *
> >> - * Note: Currently only 0 and 180 degrees are supported.
> >> - *
> >> - * Return:
> >> - * True if the plane can do the rotation, false otherwise.
> >> - */
> >> -bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int
> >> *rotation)
> >
> > Why not static? Doesn't seem to be used by anything outside of
> > drm_client_modeset.c.
> >
>
> It is used in drm_fb_helper.c:drm_setup_crtcs_fb() to set up any
> rotation and do fbcon sw rotation if necessary. Clients that support
> rotation need to call it.
>
> >> -{
> >> - struct drm_connector *connector = modeset->connectors[0];
> >> - struct drm_plane *plane = modeset->crtc->primary;
> >> - u64 valid_mask = 0;
> >> - unsigned int i;
> >> -
> >> - if (!modeset->num_connectors)
> >> - return false;
> >> -
> >> - switch (connector->display_info.panel_orientation) {
> >> - case DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP:
> >> - *rotation = DRM_MODE_ROTATE_180;
> >> - break;
> >> - case DRM_MODE_PANEL_ORIENTATION_LEFT_UP:
> >> - *rotation = DRM_MODE_ROTATE_90;
> >> - break;
> >> - case DRM_MODE_PANEL_ORIENTATION_RIGHT_UP:
> >> - *rotation = DRM_MODE_ROTATE_270;
> >> - break;
> >> - default:
> >> - *rotation = DRM_MODE_ROTATE_0;
> >> - }
> >> -
> >> - /*
> >> - * TODO: support 90 / 270 degree hardware rotation,
> >> - * depending on the hardware this may require the framebuffer
> >> - * to be in a specific tiling format.
> >> - */
> >> - if (*rotation != DRM_MODE_ROTATE_180 || !plane->rotation_property)
> >> - return false;
> >> -
> >> - for (i = 0; i < plane->rotation_property->num_values; i++)
> >> - valid_mask |= (1ULL << plane->rotation_property->values[i]);
> >> -
> >> - if (!(*rotation & valid_mask))
> >> - return false;
> >> -
> >> - return true;
> >> -}
> >> -
>
> <snip>
>
> >> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> >> index 858c8be70870..64b725b318f2 100644
> >> --- a/include/drm/drm_client.h
> >> +++ b/include/drm/drm_client.h
> >> @@ -154,6 +154,10 @@ int drm_client_modeset_create(struct drm_client_dev
> >> *client);
> >> void drm_client_modeset_free(struct drm_client_dev *client);
> >> void drm_client_modeset_release(struct drm_client_dev *client);
> >> struct drm_mode_set *drm_client_find_modeset(struct drm_client_dev
> >> *client, struct drm_crtc *crtc);
> >> +bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int
> >> *rotation);
> >> +int drm_client_modeset_commit_force(struct drm_client_dev *client);
> >
> > I think latest here the _force postfix stopped making sense. It's just a
> > commit. Also I'm wondering whether we shouldn't pull the
> > master_acquire_internal into these helpers here, there's not really a
> > use-case I can think of where we should not check for other masters.
> >
>
> drm_master_internal_acquire() is used in various places in drm_fb_helper
> for functions that doesn't make sense to move to drm_client, like:
> - drm_fb_helper_setcmap
> - drm_fb_helper_ioctl
> - drm_fb_helper_pan_display
See discussion on the earlier patches, I completely backtracked on this
after better understanding why we need _force.
And exporting/using master_acquire_internal by drm_clients makes total
sense to me.
-Daniel
>
> Noralf.
>
> > Only two kernel modeset requests want to ignore master status:
> > - debug enter/leave, which is utterly broken by design (and outright
> > disable for any atomic driver)
> > - panic handling, for which we now have a really nice plan, plus first
> > sketches of an implementation.
> >
> > Cheers, Daniel
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
