[Intel-gfx] [PATCH v2 08/12] drm/fb-helper: Move out commit code

[Noralf Trønnes]

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 <noralf at 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

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
>