Re: [PATCH 1/2] drm/fb-helper: Make fbdev inherit the crtc's rotation

[Bastien Nocera <hadess@hadess.net>]

On Sun, 2017-04-30 at 21:22 +0200, Hans de Goede wrote:
> Hi,
> 
> On 26-04-17 14:13, Bastien Nocera wrote:
> > On Sun, 2017-04-23 at 18:11 +0200, Hans de Goede wrote:
> > > From: Ville Syrjala <ville.syrjala@linux.intel.com>
> > > 
> > > If a connector added through drm_fb_helper_add_one_connector()
> > > has
> > > a crtc attached and that crtc has a rotation configured make the
> > > fbdev inherit the crtc's rotation.
> > > 
> > > This is useful on e.g. some tablets which have their lcd panel
> > > mounted
> > > upside down, which before this commit would result in the kernel
> > > boot
> > > messages switching from being shown the right way up in efifb to
> > > being
> > > shown upside down as soon as a native kms driver loads.
> > > 
> > > BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=94894
> > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > > [hdegoede@redhat.com: Split the drm/fb-helper bits out of Ville's
> > >   "drm/fb-helper: Inherit rotation wip" patch]
> > > Tested-by: Hans de Goede <hdegoede@redhat.com>
> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > > ---
> > >   drivers/gpu/drm/drm_fb_helper.c | 51
> > > ++++++++++++++++++++++++++++++++++++++---
> > >   include/drm/drm_fb_helper.h     |  2 ++
> > >   2 files changed, 50 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > > b/drivers/gpu/drm/drm_fb_helper.c
> > > index 324a688..c97e00ab 100644
> > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > @@ -159,6 +159,8 @@ int drm_fb_helper_add_one_connector(struct
> > > drm_fb_helper *fb_helper, struct drm_
> > >   {
> > >   	struct drm_fb_helper_connector **temp;
> > >   	struct drm_fb_helper_connector *fb_helper_connector;
> > > +	struct drm_crtc *crtc = connector->encoder ?
> > > +				connector->encoder->crtc : NULL;
> > >   
> > >   	if (!drm_fbdev_emulation)
> > >   		return 0;
> > > @@ -180,6 +182,11 @@ int drm_fb_helper_add_one_connector(struct
> > > drm_fb_helper *fb_helper, struct drm_
> > >   
> > >   	drm_connector_reference(connector);
> > >   	fb_helper_connector->connector = connector;
> > > +	if (crtc && crtc->primary->state)
> > > +		fb_helper_connector->rotation = crtc->primary-
> > > > state->rotation;
> > > 
> > > +	if (!fb_helper_connector->rotation)
> > > +		fb_helper_connector->rotation = DRM_ROTATE_0;
> > 
> > That's equivalent to:
> > if (fb_helper_connector->rotation = DRM_ROTATE_0)
> >     fb_helper_connector->rotation = DRM_ROTATE_0;
> 
> No it is not equivalent:
> 
> include/drm/drm_blend.h
> 40:#define DRM_ROTATE_0 BIT(0)
> 
> and BIT(0) is (1 << 0)

Urgh. I'd have a "DRM_ROTATE_UNSET = 0" added either to the public
header, or to this file and compare to it instead of "!rotation".
Because this really wasn't clear or obvious.

Cheers
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx