Message ID | 20191206124713.5748-5-tzimmermann@suse.de (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | drm/udl: Convert to simple-pipe helpers and clean up | expand |
On Fri, 6 Dec 2019 at 12:47, Thomas Zimmermann <tzimmermann@suse.de> wrote: > > DPMS functionality is only used by the CRTC's enable and disable > functions. Inline the code. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/udl/udl_modeset.c | 84 +++++++++++-------------------- > 1 file changed, 30 insertions(+), 54 deletions(-) > > diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c > index 72884cbda131..4681446c2323 100644 > --- a/drivers/gpu/drm/udl/udl_modeset.c > +++ b/drivers/gpu/drm/udl/udl_modeset.c > @@ -48,25 +48,9 @@ static char *udl_vidreg_unlock(char *buf) > * 0x01 H and V sync off (screen blank but powered) > * 0x07 DPMS powerdown (requires modeset to come back) > */ > -static char *udl_set_blank(char *buf, int dpms_mode) > +static char *udl_set_blank_mode(char *buf, u8 mode) > { > - u8 reg; > - switch (dpms_mode) { > - case DRM_MODE_DPMS_OFF: > - reg = 0x07; > - break; > - case DRM_MODE_DPMS_STANDBY: > - reg = 0x05; > - break; > - case DRM_MODE_DPMS_SUSPEND: > - reg = 0x01; > - break; > - case DRM_MODE_DPMS_ON: > - reg = 0x00; > - break; > - } > - As a follow-up, please add/use symbolic names for the the four states. Apart from the cosmetic aspect, this allows us to trivially change from DPMS_OFF to DPMS_SUSPEND or DPMS_STANDBY at any random point. As-is the series is: Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Thanks Emil
Hi Am 09.12.19 um 15:35 schrieb Emil Velikov: > On Fri, 6 Dec 2019 at 12:47, Thomas Zimmermann <tzimmermann@suse.de> wrote: >> >> DPMS functionality is only used by the CRTC's enable and disable >> functions. Inline the code. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> --- >> drivers/gpu/drm/udl/udl_modeset.c | 84 +++++++++++-------------------- >> 1 file changed, 30 insertions(+), 54 deletions(-) >> >> diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c >> index 72884cbda131..4681446c2323 100644 >> --- a/drivers/gpu/drm/udl/udl_modeset.c >> +++ b/drivers/gpu/drm/udl/udl_modeset.c >> @@ -48,25 +48,9 @@ static char *udl_vidreg_unlock(char *buf) >> * 0x01 H and V sync off (screen blank but powered) >> * 0x07 DPMS powerdown (requires modeset to come back) >> */ >> -static char *udl_set_blank(char *buf, int dpms_mode) >> +static char *udl_set_blank_mode(char *buf, u8 mode) >> { >> - u8 reg; >> - switch (dpms_mode) { >> - case DRM_MODE_DPMS_OFF: >> - reg = 0x07; >> - break; >> - case DRM_MODE_DPMS_STANDBY: >> - reg = 0x05; >> - break; >> - case DRM_MODE_DPMS_SUSPEND: >> - reg = 0x01; >> - break; >> - case DRM_MODE_DPMS_ON: >> - reg = 0x00; >> - break; >> - } >> - > As a follow-up, please add/use symbolic names for the the four states. > Apart from the cosmetic aspect, this allows us to trivially change > from DPMS_OFF to DPMS_SUSPEND or DPMS_STANDBY at any random point. > > As-is the series is: > Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Thanks. The symbolic names are trivial to add here. I'll put them into v3 of this patchset. Best regards Thomas > > Thanks > Emil >
diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c index 72884cbda131..4681446c2323 100644 --- a/drivers/gpu/drm/udl/udl_modeset.c +++ b/drivers/gpu/drm/udl/udl_modeset.c @@ -48,25 +48,9 @@ static char *udl_vidreg_unlock(char *buf) * 0x01 H and V sync off (screen blank but powered) * 0x07 DPMS powerdown (requires modeset to come back) */ -static char *udl_set_blank(char *buf, int dpms_mode) +static char *udl_set_blank_mode(char *buf, u8 mode) { - u8 reg; - switch (dpms_mode) { - case DRM_MODE_DPMS_OFF: - reg = 0x07; - break; - case DRM_MODE_DPMS_STANDBY: - reg = 0x05; - break; - case DRM_MODE_DPMS_SUSPEND: - reg = 0x01; - break; - case DRM_MODE_DPMS_ON: - reg = 0x00; - break; - } - - return udl_set_register(buf, 0x1f, reg); + return udl_set_register(buf, 0x1f, mode); } static char *udl_set_color_depth(char *buf, u8 selection) @@ -237,6 +221,11 @@ static int udl_crtc_write_mode_to_hw(struct drm_crtc *crtc) char *buf; int retval; + if (udl->mode_buf_len == 0) { + DRM_ERROR("No mode set\n"); + return -EINVAL; + } + urb = udl_get_urb(dev); if (!urb) return -ENOMEM; @@ -249,38 +238,6 @@ static int udl_crtc_write_mode_to_hw(struct drm_crtc *crtc) return retval; } - -static void udl_crtc_dpms(struct drm_crtc *crtc, int mode) -{ - struct drm_device *dev = crtc->dev; - struct udl_device *udl = dev->dev_private; - int retval; - - if (mode == DRM_MODE_DPMS_OFF) { - char *buf; - struct urb *urb; - urb = udl_get_urb(dev); - if (!urb) - return; - - buf = (char *)urb->transfer_buffer; - buf = udl_vidreg_lock(buf); - buf = udl_set_blank(buf, mode); - buf = udl_vidreg_unlock(buf); - - buf = udl_dummy_render(buf); - retval = udl_submit_urb(dev, urb, buf - (char *) - urb->transfer_buffer); - } else { - if (udl->mode_buf_len == 0) { - DRM_ERROR("Trying to enable DPMS with no mode\n"); - return; - } - udl_crtc_write_mode_to_hw(crtc); - } - -} - /* * Simple display pipeline */ @@ -311,6 +268,8 @@ udl_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, char *wrptr; int color_depth = UDL_COLOR_DEPTH_16BPP; + crtc_state->no_vblank = true; + udl->crtc = crtc; buf = (char *)udl->mode_buf; @@ -327,7 +286,7 @@ udl_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, wrptr = udl_set_base8bpp(wrptr, 2 * mode->vdisplay * mode->hdisplay); wrptr = udl_set_vid_cmds(wrptr, mode); - wrptr = udl_set_blank(wrptr, DRM_MODE_DPMS_ON); + wrptr = udl_set_blank_mode(wrptr, 0x00); wrptr = udl_vidreg_unlock(wrptr); wrptr = udl_dummy_render(wrptr); @@ -339,15 +298,32 @@ udl_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, udl_handle_damage(fb, 0, 0, fb->width, fb->height); - udl_crtc_dpms(&pipe->crtc, DRM_MODE_DPMS_ON); + if (!crtc_state->mode_changed) + return; - crtc_state->no_vblank = true; + /* enable display */ + udl_crtc_write_mode_to_hw(crtc); } static void udl_simple_display_pipe_disable(struct drm_simple_display_pipe *pipe) { - udl_crtc_dpms(&pipe->crtc, DRM_MODE_DPMS_OFF); + struct drm_crtc *crtc= &pipe->crtc; + struct drm_device *dev = crtc->dev; + struct urb *urb; + char *buf; + + urb = udl_get_urb(dev); + if (!urb) + return; + + buf = (char *)urb->transfer_buffer; + buf = udl_vidreg_lock(buf); + buf = udl_set_blank_mode(buf, 0x07); + buf = udl_vidreg_unlock(buf); + buf = udl_dummy_render(buf); + + udl_submit_urb(dev, urb, buf - (char *)urb->transfer_buffer); } static int
DPMS functionality is only used by the CRTC's enable and disable functions. Inline the code. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/udl/udl_modeset.c | 84 +++++++++++-------------------- 1 file changed, 30 insertions(+), 54 deletions(-)