| Message ID | 20161213181912.92904-1-daniels@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Dec 13, 2016 at 06:19:12PM +0000, Daniel Stone wrote: > If atomic state is available, use this to read the current plane in > GetCrtc/GetPlane, rather than the legacy points. > > Signed-off-by: Daniel Stone <daniels@collabora.com> > Cc: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/drm_crtc.c | 5 ++++- > drivers/gpu/drm/drm_plane.c | 8 ++++++-- > 2 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index e75f62c..14c5807 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -357,7 +357,10 @@ int drm_mode_getcrtc(struct drm_device *dev, > > drm_modeset_lock_crtc(crtc, crtc->primary); > crtc_resp->gamma_size = crtc->gamma_size; > - if (crtc->primary->fb) > + > + if (crtc->primary->state && crtc->primary->state->fb) > + crtc_resp->fb_id = crtc->primary->state->fb->base.id; > + else if (!crtc->primary->state && crtc->primary->fb) > crtc_resp->fb_id = crtc->primary->fb->base.id; I think what we do elsewhere is totally ignore the legacy junk if the ->state pointer exists. > else > crtc_resp->fb_id = 0; > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > index 62b98f3..c2dc8e6 100644 > --- a/drivers/gpu/drm/drm_plane.c > +++ b/drivers/gpu/drm/drm_plane.c > @@ -392,12 +392,16 @@ int drm_mode_getplane(struct drm_device *dev, void *data, > return -ENOENT; > > drm_modeset_lock(&plane->mutex, NULL); > - if (plane->crtc) > + if (plane->state && plane->state->crtc) > + plane_resp->crtc_id = plane->state->crtc->base.id; > + else if (!plane->state && plane->crtc) > plane_resp->crtc_id = plane->crtc->base.id; > else > plane_resp->crtc_id = 0; > > - if (plane->fb) > + if (plane->state && plane->state->fb) > + plane_resp->fb_id = plane->state->fb->base.id; > + else if (!plane->state && plane->fb) > plane_resp->fb_id = plane->fb->base.id; > else > plane_resp->fb_id = 0; > -- > 2.10.2 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi, > On 13 Dec 2016, at 6:48 pm, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > >> On Tue, Dec 13, 2016 at 06:19:12PM +0000, Daniel Stone wrote: >> @@ -357,7 +357,10 @@ int drm_mode_getcrtc(struct drm_device *dev, >> >> drm_modeset_lock_crtc(crtc, crtc->primary); >> crtc_resp->gamma_size = crtc->gamma_size; >> - if (crtc->primary->fb) >> + >> + if (crtc->primary->state && crtc->primary->state->fb) >> + crtc_resp->fb_id = crtc->primary->state->fb->base.id; >> + else if (!crtc->primary->state && crtc->primary->fb) >> crtc_resp->fb_id = crtc->primary->fb->base.id; > > I think what we do elsewhere is totally ignore the legacy junk if the > ->state pointer exists. Indeed, hence the negative state check on the second branch; having nested if statements instead seemed unnecessarily unwieldy, but the effect is the same. [Obligatory mobile formatting apology.] Cheers, Daniel
On Tue, Dec 13, 2016 at 07:02:53PM +0000, Daniel Stone wrote: > Hi, > > > On 13 Dec 2016, at 6:48 pm, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > > >> On Tue, Dec 13, 2016 at 06:19:12PM +0000, Daniel Stone wrote: > >> @@ -357,7 +357,10 @@ int drm_mode_getcrtc(struct drm_device *dev, > >> > >> drm_modeset_lock_crtc(crtc, crtc->primary); > >> crtc_resp->gamma_size = crtc->gamma_size; > >> - if (crtc->primary->fb) > >> + > >> + if (crtc->primary->state && crtc->primary->state->fb) > >> + crtc_resp->fb_id = crtc->primary->state->fb->base.id; > >> + else if (!crtc->primary->state && crtc->primary->fb) > >> crtc_resp->fb_id = crtc->primary->fb->base.id; > > > > I think what we do elsewhere is totally ignore the legacy junk if the > > ->state pointer exists. > > Indeed, hence the negative state check on the second branch; having nested if statements instead seemed unnecessarily unwieldy, but the effect is the same. My bad. You hid it well though ;) Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
On Tue, Dec 13, 2016 at 11:14:47PM +0200, Ville Syrjälä wrote: > On Tue, Dec 13, 2016 at 07:02:53PM +0000, Daniel Stone wrote: > > Hi, > > > > > On 13 Dec 2016, at 6:48 pm, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > > > > >> On Tue, Dec 13, 2016 at 06:19:12PM +0000, Daniel Stone wrote: > > >> @@ -357,7 +357,10 @@ int drm_mode_getcrtc(struct drm_device *dev, > > >> > > >> drm_modeset_lock_crtc(crtc, crtc->primary); > > >> crtc_resp->gamma_size = crtc->gamma_size; > > >> - if (crtc->primary->fb) > > >> + > > >> + if (crtc->primary->state && crtc->primary->state->fb) > > >> + crtc_resp->fb_id = crtc->primary->state->fb->base.id; > > >> + else if (!crtc->primary->state && crtc->primary->fb) > > >> crtc_resp->fb_id = crtc->primary->fb->base.id; > > > > > > I think what we do elsewhere is totally ignore the legacy junk if the > > > ->state pointer exists. > > > > Indeed, hence the negative state check on the second branch; having nested if statements instead seemed unnecessarily unwieldy, but the effect is the same. > > My bad. You hid it well though ;) > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Applied to drm-misc, thanks. -Daniel
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index e75f62c..14c5807 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -357,7 +357,10 @@ int drm_mode_getcrtc(struct drm_device *dev, drm_modeset_lock_crtc(crtc, crtc->primary); crtc_resp->gamma_size = crtc->gamma_size; - if (crtc->primary->fb) + + if (crtc->primary->state && crtc->primary->state->fb) + crtc_resp->fb_id = crtc->primary->state->fb->base.id; + else if (!crtc->primary->state && crtc->primary->fb) crtc_resp->fb_id = crtc->primary->fb->base.id; else crtc_resp->fb_id = 0; diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index 62b98f3..c2dc8e6 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -392,12 +392,16 @@ int drm_mode_getplane(struct drm_device *dev, void *data, return -ENOENT; drm_modeset_lock(&plane->mutex, NULL); - if (plane->crtc) + if (plane->state && plane->state->crtc) + plane_resp->crtc_id = plane->state->crtc->base.id; + else if (!plane->state && plane->crtc) plane_resp->crtc_id = plane->crtc->base.id; else plane_resp->crtc_id = 0; - if (plane->fb) + if (plane->state && plane->state->fb) + plane_resp->fb_id = plane->state->fb->base.id; + else if (!plane->state && plane->fb) plane_resp->fb_id = plane->fb->base.id; else plane_resp->fb_id = 0;
If atomic state is available, use this to read the current plane in GetCrtc/GetPlane, rather than the legacy points. Signed-off-by: Daniel Stone <daniels@collabora.com> Cc: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/drm_crtc.c | 5 ++++- drivers/gpu/drm/drm_plane.c | 8 ++++++-- 2 files changed, 10 insertions(+), 3 deletions(-)