Message ID | 20191014140416.28517-10-tzimmermann@suse.de (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | DRM fbconv helpers for converting fbdev drivers | expand |
Hi Thomas, On Mon, Oct 14, 2019 at 4:05 PM Thomas Zimmermann <tzimmermann@suse.de> wrote: > The display mode is set by converting the DRM display mode to an > fb_info state and handling it to the fbdev driver's fb_setvar() > function. This also requires a color depth, which we take from the > value of struct drm_mode_config.preferred_depth > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- a/drivers/gpu/drm/drm_fbconv_helper.c > +++ b/drivers/gpu/drm/drm_fbconv_helper.c > @@ -919,6 +919,24 @@ static int drm_fbconv_update_fb_var_screeninfo_from_framebuffer( > return 0; > } > > +static int drm_fbconv_update_fb_var_screeninfo_from_simple_display_pipe( > + struct fb_var_screeninfo *fb_var, struct drm_simple_display_pipe *pipe) > +{ > + struct drm_plane *primary = pipe->crtc.primary; > + struct drm_fbconv_modeset *modeset = drm_fbconv_modeset_of_pipe(pipe); > + > + if (primary && primary->state && primary->state->fb) > + return drm_fbconv_update_fb_var_screeninfo_from_framebuffer( > + fb_var, primary->state->fb, > + modeset->fb_info->fix.smem_len); > + > + fb_var->xres_virtual = fb_var->xres; > + fb_var->yres_virtual = fb_var->yres; > + fb_var->bits_per_pixel = modeset->dev->mode_config.preferred_depth; This looks wrong to me: IMHO bits_per_pixel should be derived from the fourcc format of the _new_ mode to be set... > + > + return 0; > +} > + > /** > * drm_fbconv_simple_display_pipe_mode_valid - default implementation for > * struct drm_simple_display_pipe_funcs.mode_valid > @@ -950,6 +968,28 @@ bool drm_fbconv_simple_display_pipe_mode_fixup( > struct drm_crtc *crtc, const struct drm_display_mode *mode, > struct drm_display_mode *adjusted_mode) > { > + struct drm_simple_display_pipe *pipe = > + container_of(crtc, struct drm_simple_display_pipe, crtc); > + struct drm_fbconv_modeset *modeset = drm_fbconv_modeset_of_pipe(pipe); > + struct fb_var_screeninfo fb_var; > + int ret; > + > + if (!modeset->fb_info->fbops->fb_check_var) > + return true; > + > + drm_fbconv_init_fb_var_screeninfo_from_mode(&fb_var, mode); > + > + ret = drm_fbconv_update_fb_var_screeninfo_from_simple_display_pipe( > + &fb_var, &modeset->pipe); > + if (ret) > + return true; > + > + ret = modeset->fb_info->fbops->fb_check_var(&fb_var, modeset->fb_info); ... hence this fails if the requested mode is valid with the new fourcc format, but invalid with the old (but preferred) depth. E.g. due to bandwidth limitations, a high-resolution mode is valid with a low color depth, while a high color depth is limited to lower resolutions. Unfortunately we do not know the new fourcc format here, as both drm_simple_display_pipe_funcs.mode_{valid,fixup}() are passed the mode (from drm_mode_set.mode), but not the new format (from drm_mode_set.fb->format). Am I missing something? Is the new format available in some other way? Thanks! > + if (ret < 0) > + return false; > + > + drm_mode_update_from_fb_var_screeninfo(adjusted_mode, &fb_var); > + > return true; > } > EXPORT_SYMBOL(drm_fbconv_simple_display_pipe_mode_fixup); Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, first of all, thanks for looking at the patch. Am 28.05.22 um 22:17 schrieb Geert Uytterhoeven: > Hi Thomas, > > On Mon, Oct 14, 2019 at 4:05 PM Thomas Zimmermann <tzimmermann@suse.de> wrote: >> The display mode is set by converting the DRM display mode to an >> fb_info state and handling it to the fbdev driver's fb_setvar() >> function. This also requires a color depth, which we take from the >> value of struct drm_mode_config.preferred_depth >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > >> --- a/drivers/gpu/drm/drm_fbconv_helper.c >> +++ b/drivers/gpu/drm/drm_fbconv_helper.c >> @@ -919,6 +919,24 @@ static int drm_fbconv_update_fb_var_screeninfo_from_framebuffer( >> return 0; >> } >> >> +static int drm_fbconv_update_fb_var_screeninfo_from_simple_display_pipe( >> + struct fb_var_screeninfo *fb_var, struct drm_simple_display_pipe *pipe) >> +{ >> + struct drm_plane *primary = pipe->crtc.primary; >> + struct drm_fbconv_modeset *modeset = drm_fbconv_modeset_of_pipe(pipe); >> + >> + if (primary && primary->state && primary->state->fb) >> + return drm_fbconv_update_fb_var_screeninfo_from_framebuffer( >> + fb_var, primary->state->fb, >> + modeset->fb_info->fix.smem_len); >> + >> + fb_var->xres_virtual = fb_var->xres; >> + fb_var->yres_virtual = fb_var->yres; >> + fb_var->bits_per_pixel = modeset->dev->mode_config.preferred_depth; > > This looks wrong to me: IMHO bits_per_pixel should be derived from > the fourcc format of the _new_ mode to be set... Indeed, this appears to be wrong. > >> + >> + return 0; >> +} >> + >> /** >> * drm_fbconv_simple_display_pipe_mode_valid - default implementation for >> * struct drm_simple_display_pipe_funcs.mode_valid >> @@ -950,6 +968,28 @@ bool drm_fbconv_simple_display_pipe_mode_fixup( >> struct drm_crtc *crtc, const struct drm_display_mode *mode, >> struct drm_display_mode *adjusted_mode) >> { >> + struct drm_simple_display_pipe *pipe = >> + container_of(crtc, struct drm_simple_display_pipe, crtc); >> + struct drm_fbconv_modeset *modeset = drm_fbconv_modeset_of_pipe(pipe); >> + struct fb_var_screeninfo fb_var; >> + int ret; >> + >> + if (!modeset->fb_info->fbops->fb_check_var) >> + return true; >> + >> + drm_fbconv_init_fb_var_screeninfo_from_mode(&fb_var, mode); >> + >> + ret = drm_fbconv_update_fb_var_screeninfo_from_simple_display_pipe( >> + &fb_var, &modeset->pipe); >> + if (ret) >> + return true; >> + >> + ret = modeset->fb_info->fbops->fb_check_var(&fb_var, modeset->fb_info); > > ... hence this fails if the requested mode is valid with the new > fourcc format, but invalid with the old (but preferred) depth. > E.g. due to bandwidth limitations, a high-resolution mode is valid > with a low color depth, while a high color depth is limited to lower > resolutions. I tested the helpers with various fbdev drivers and modified them until all of these drivers produced at least some output. I'm not surprised that there are still bugs. > > Unfortunately we do not know the new fourcc format here, as both > drm_simple_display_pipe_funcs.mode_{valid,fixup}() are passed > the mode (from drm_mode_set.mode), but not the new format (from > drm_mode_set.fb->format). > > Am I missing something? Is the new format available in some other way? We can always get the format from the new plane state of modeset->pipe->plane. We'd have this in the atomic_check call. And it appears that drm_fbconv_simple_display_pipe_check() is a better place for this code anyway. Best regards Thomas > Thanks! > >> + if (ret < 0) >> + return false; >> + >> + drm_mode_update_from_fb_var_screeninfo(adjusted_mode, &fb_var); >> + >> return true; >> } >> EXPORT_SYMBOL(drm_fbconv_simple_display_pipe_mode_fixup); > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
Hi Thomas, On Mon, May 30, 2022 at 9:47 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: > first of all, thanks for looking at the patch. Thank you, your patch series helped a lot. > Am 28.05.22 um 22:17 schrieb Geert Uytterhoeven: > > On Mon, Oct 14, 2019 at 4:05 PM Thomas Zimmermann <tzimmermann@suse.de> wrote: > >> The display mode is set by converting the DRM display mode to an > >> fb_info state and handling it to the fbdev driver's fb_setvar() > >> function. This also requires a color depth, which we take from the > >> value of struct drm_mode_config.preferred_depth > >> > >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > > > >> --- a/drivers/gpu/drm/drm_fbconv_helper.c > >> +++ b/drivers/gpu/drm/drm_fbconv_helper.c > >> @@ -919,6 +919,24 @@ static int drm_fbconv_update_fb_var_screeninfo_from_framebuffer( > >> return 0; > >> } > >> > >> +static int drm_fbconv_update_fb_var_screeninfo_from_simple_display_pipe( > >> + struct fb_var_screeninfo *fb_var, struct drm_simple_display_pipe *pipe) > >> +{ > >> + struct drm_plane *primary = pipe->crtc.primary; > >> + struct drm_fbconv_modeset *modeset = drm_fbconv_modeset_of_pipe(pipe); > >> + > >> + if (primary && primary->state && primary->state->fb) > >> + return drm_fbconv_update_fb_var_screeninfo_from_framebuffer( > >> + fb_var, primary->state->fb, > >> + modeset->fb_info->fix.smem_len); > >> + > >> + fb_var->xres_virtual = fb_var->xres; > >> + fb_var->yres_virtual = fb_var->yres; > >> + fb_var->bits_per_pixel = modeset->dev->mode_config.preferred_depth; > > > > This looks wrong to me: IMHO bits_per_pixel should be derived from > > the fourcc format of the _new_ mode to be set... > > Indeed, this appears to be wrong. OK. > > > > >> + > >> + return 0; > >> +} > >> + > >> /** > >> * drm_fbconv_simple_display_pipe_mode_valid - default implementation for > >> * struct drm_simple_display_pipe_funcs.mode_valid > >> @@ -950,6 +968,28 @@ bool drm_fbconv_simple_display_pipe_mode_fixup( > >> struct drm_crtc *crtc, const struct drm_display_mode *mode, > >> struct drm_display_mode *adjusted_mode) > >> { > >> + struct drm_simple_display_pipe *pipe = > >> + container_of(crtc, struct drm_simple_display_pipe, crtc); > >> + struct drm_fbconv_modeset *modeset = drm_fbconv_modeset_of_pipe(pipe); > >> + struct fb_var_screeninfo fb_var; > >> + int ret; > >> + > >> + if (!modeset->fb_info->fbops->fb_check_var) > >> + return true; > >> + > >> + drm_fbconv_init_fb_var_screeninfo_from_mode(&fb_var, mode); > >> + > >> + ret = drm_fbconv_update_fb_var_screeninfo_from_simple_display_pipe( > >> + &fb_var, &modeset->pipe); > >> + if (ret) > >> + return true; > >> + > >> + ret = modeset->fb_info->fbops->fb_check_var(&fb_var, modeset->fb_info); > > > > ... hence this fails if the requested mode is valid with the new > > fourcc format, but invalid with the old (but preferred) depth. > > E.g. due to bandwidth limitations, a high-resolution mode is valid > > with a low color depth, while a high color depth is limited to lower > > resolutions. > > I tested the helpers with various fbdev drivers and modified them until > all of these drivers produced at least some output. I'm not surprised > that there are still bugs. As usual, the devil is in the details ;-) The other issue I was facing are the non-rounding KHZ2PICOS() and PICOS2KHZ() macros, and the numerous back-and-forth conversions: a valid pixclock in kHz is converted to a valid pixclock in ps, and accepted. The returned pixclock in ps is slightly different, and converted to an invalid pixclock in kHz, hence rejected in the next iteration (remember: fb_ops.fb_check_var() should only round up values to match, never round down)... > > Unfortunately we do not know the new fourcc format here, as both > > drm_simple_display_pipe_funcs.mode_{valid,fixup}() are passed > > the mode (from drm_mode_set.mode), but not the new format (from > > drm_mode_set.fb->format). > > > > Am I missing something? Is the new format available in some other way? > > We can always get the format from the new plane state of > modeset->pipe->plane. We'd have this in the atomic_check call. And it > appears that drm_fbconv_simple_display_pipe_check() is a better place > for this code anyway. Thanks, I'll give that a try! Anyway, I finally made some progress with KMS-style mode-setting inside my ataridrm driver. Before, I relied solely on initial fbdev-style mode-setting in the probe function. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Thomas, On Mon, May 30, 2022 at 10:34 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Mon, May 30, 2022 at 9:47 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: > > Am 28.05.22 um 22:17 schrieb Geert Uytterhoeven: > > > On Mon, Oct 14, 2019 at 4:05 PM Thomas Zimmermann <tzimmermann@suse.de> wrote: > > >> The display mode is set by converting the DRM display mode to an > > >> fb_info state and handling it to the fbdev driver's fb_setvar() > > >> function. This also requires a color depth, which we take from the > > >> value of struct drm_mode_config.preferred_depth > > >> > > >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > > > > > >> --- a/drivers/gpu/drm/drm_fbconv_helper.c > > >> +++ b/drivers/gpu/drm/drm_fbconv_helper.c > > >> @@ -919,6 +919,24 @@ static int drm_fbconv_update_fb_var_screeninfo_from_framebuffer( > > >> return 0; > > >> } > > >> > > >> +static int drm_fbconv_update_fb_var_screeninfo_from_simple_display_pipe( > > >> + struct fb_var_screeninfo *fb_var, struct drm_simple_display_pipe *pipe) > > >> +{ > > >> + struct drm_plane *primary = pipe->crtc.primary; > > >> + struct drm_fbconv_modeset *modeset = drm_fbconv_modeset_of_pipe(pipe); > > >> + > > >> + if (primary && primary->state && primary->state->fb) > > >> + return drm_fbconv_update_fb_var_screeninfo_from_framebuffer( > > >> + fb_var, primary->state->fb, > > >> + modeset->fb_info->fix.smem_len); > > >> + > > >> + fb_var->xres_virtual = fb_var->xres; > > >> + fb_var->yres_virtual = fb_var->yres; > > >> + fb_var->bits_per_pixel = modeset->dev->mode_config.preferred_depth; > > > > > > This looks wrong to me: IMHO bits_per_pixel should be derived from > > > the fourcc format of the _new_ mode to be set... > > > > Indeed, this appears to be wrong. > > OK. > > > > > > > > >> + > > >> + return 0; > > >> +} > > >> + > > >> /** > > >> * drm_fbconv_simple_display_pipe_mode_valid - default implementation for > > >> * struct drm_simple_display_pipe_funcs.mode_valid > > >> @@ -950,6 +968,28 @@ bool drm_fbconv_simple_display_pipe_mode_fixup( > > >> struct drm_crtc *crtc, const struct drm_display_mode *mode, > > >> struct drm_display_mode *adjusted_mode) > > >> { > > >> + struct drm_simple_display_pipe *pipe = > > >> + container_of(crtc, struct drm_simple_display_pipe, crtc); > > >> + struct drm_fbconv_modeset *modeset = drm_fbconv_modeset_of_pipe(pipe); > > >> + struct fb_var_screeninfo fb_var; > > >> + int ret; > > >> + > > >> + if (!modeset->fb_info->fbops->fb_check_var) > > >> + return true; > > >> + > > >> + drm_fbconv_init_fb_var_screeninfo_from_mode(&fb_var, mode); > > >> + > > >> + ret = drm_fbconv_update_fb_var_screeninfo_from_simple_display_pipe( > > >> + &fb_var, &modeset->pipe); > > >> + if (ret) > > >> + return true; > > >> + > > >> + ret = modeset->fb_info->fbops->fb_check_var(&fb_var, modeset->fb_info); > > > > > > ... hence this fails if the requested mode is valid with the new > > > fourcc format, but invalid with the old (but preferred) depth. > > > E.g. due to bandwidth limitations, a high-resolution mode is valid > > > with a low color depth, while a high color depth is limited to lower > > > resolutions. > > > Unfortunately we do not know the new fourcc format here, as both > > > drm_simple_display_pipe_funcs.mode_{valid,fixup}() are passed > > > the mode (from drm_mode_set.mode), but not the new format (from > > > drm_mode_set.fb->format). > > > > > > Am I missing something? Is the new format available in some other way? > > > > We can always get the format from the new plane state of > > modeset->pipe->plane. We'd have this in the atomic_check call. And it > > appears that drm_fbconv_simple_display_pipe_check() is a better place > > for this code anyway. > > Thanks, I'll give that a try! Getting the format from the new plane state of pipe->plane doesn't work, as pipe->plane.state->fb = NULL. But it is indeed available in the drm_simple_display_pipe_funcs.check() callback, so that seems to work... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
diff --git a/drivers/gpu/drm/drm_fbconv_helper.c b/drivers/gpu/drm/drm_fbconv_helper.c index cf218016ac05..ca8b43c91266 100644 --- a/drivers/gpu/drm/drm_fbconv_helper.c +++ b/drivers/gpu/drm/drm_fbconv_helper.c @@ -919,6 +919,24 @@ static int drm_fbconv_update_fb_var_screeninfo_from_framebuffer( return 0; } +static int drm_fbconv_update_fb_var_screeninfo_from_simple_display_pipe( + struct fb_var_screeninfo *fb_var, struct drm_simple_display_pipe *pipe) +{ + struct drm_plane *primary = pipe->crtc.primary; + struct drm_fbconv_modeset *modeset = drm_fbconv_modeset_of_pipe(pipe); + + if (primary && primary->state && primary->state->fb) + return drm_fbconv_update_fb_var_screeninfo_from_framebuffer( + fb_var, primary->state->fb, + modeset->fb_info->fix.smem_len); + + fb_var->xres_virtual = fb_var->xres; + fb_var->yres_virtual = fb_var->yres; + fb_var->bits_per_pixel = modeset->dev->mode_config.preferred_depth; + + return 0; +} + /** * drm_fbconv_simple_display_pipe_mode_valid - default implementation for * struct drm_simple_display_pipe_funcs.mode_valid @@ -950,6 +968,28 @@ bool drm_fbconv_simple_display_pipe_mode_fixup( struct drm_crtc *crtc, const struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode) { + struct drm_simple_display_pipe *pipe = + container_of(crtc, struct drm_simple_display_pipe, crtc); + struct drm_fbconv_modeset *modeset = drm_fbconv_modeset_of_pipe(pipe); + struct fb_var_screeninfo fb_var; + int ret; + + if (!modeset->fb_info->fbops->fb_check_var) + return true; + + drm_fbconv_init_fb_var_screeninfo_from_mode(&fb_var, mode); + + ret = drm_fbconv_update_fb_var_screeninfo_from_simple_display_pipe( + &fb_var, &modeset->pipe); + if (ret) + return true; + + ret = modeset->fb_info->fbops->fb_check_var(&fb_var, modeset->fb_info); + if (ret < 0) + return false; + + drm_mode_update_from_fb_var_screeninfo(adjusted_mode, &fb_var); + return true; } EXPORT_SYMBOL(drm_fbconv_simple_display_pipe_mode_fixup); @@ -1000,6 +1040,32 @@ int drm_fbconv_blit_rect(void *dst, void *vaddr, struct drm_framebuffer *fb, } EXPORT_SYMBOL(drm_fbconv_blit_rect); +/** + * drm_fbconv_blit_fullscreen - copy all pixel data from a framebuffer + * to the hardware buffer + * @dst: the on-screen hardware buffer + * @vaddr: the source buffer in kernel address space + * @fb: the framebuffer of the source buffer + * Returns: + * 0 on success, or + * a negative error code otherwise. + * + * This function is equivalent to drm_fbconv_blit_rect(), but copies the + * framebuffer's complete content. + */ +int drm_fbconv_blit_fullscreen(void *dst, void *vaddr, + struct drm_framebuffer *fb) +{ + struct drm_rect fullscreen = { + .x1 = 0, + .x2 = fb->width, + .y1 = 0, + .y2 = fb->height, + }; + return drm_fbconv_blit_rect(dst, vaddr, fb, &fullscreen); +} +EXPORT_SYMBOL(drm_fbconv_blit_fullscreen); + /** * drm_fbconv_simple_display_pipe_enable - default implementation for * struct drm_simple_display_pipe_funcs.enable @@ -1011,7 +1077,46 @@ void drm_fbconv_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, struct drm_crtc_state *crtc_state, struct drm_plane_state *plane_state) -{ } +{ + struct drm_fbconv_modeset *modeset; + struct fb_var_screeninfo fb_var; + int ret; + + /* As this is atomic mode setting, any function call is not + * allowed to fail. If it does, an additional test should be + * added to simple_display_pipe_check(). + */ + + modeset = drm_fbconv_modeset_of_pipe(pipe); + + drm_fbconv_init_fb_var_screeninfo_from_mode( + &fb_var, &crtc_state->adjusted_mode); + + if (plane_state && plane_state->fb) { + ret = drm_fbconv_update_fb_var_screeninfo_from_framebuffer( + &fb_var, plane_state->fb, + modeset->fb_info->fix.smem_len); + if (ret) + return; + } else { + fb_var.xres_virtual = fb_var.xres; + fb_var.yres_virtual = fb_var.yres; + } + + fb_var.activate = FB_ACTIVATE_NOW; + + ret = fb_set_var(modeset->fb_info, &fb_var); + if (ret) { + DRM_ERROR("fbconv: fb_set_var() failed: %d\n", ret); + return; + } + + fb_blank(modeset->fb_info, FB_BLANK_UNBLANK); + + drm_fbconv_blit_fullscreen(modeset->blit.screen_base, + modeset->blit.vmap, + plane_state->fb); +} EXPORT_SYMBOL(drm_fbconv_simple_display_pipe_enable); /** @@ -1021,7 +1126,11 @@ EXPORT_SYMBOL(drm_fbconv_simple_display_pipe_enable); */ void drm_fbconv_simple_display_pipe_disable(struct drm_simple_display_pipe *pipe) -{ } +{ + struct drm_fbconv_modeset *modeset = drm_fbconv_modeset_of_pipe(pipe); + + fb_blank(modeset->fb_info, FB_BLANK_POWERDOWN); +} EXPORT_SYMBOL(drm_fbconv_simple_display_pipe_disable); /** diff --git a/include/drm/drm_fbconv_helper.h b/include/drm/drm_fbconv_helper.h index 3e62b5e80af6..c7d211f40462 100644 --- a/include/drm/drm_fbconv_helper.h +++ b/include/drm/drm_fbconv_helper.h @@ -94,6 +94,8 @@ drm_fbconv_simple_display_pipe_cleanup_fb(struct drm_simple_display_pipe *pipe, int drm_fbconv_blit_rect(void *dst, void *vaddr, struct drm_framebuffer *fb, struct drm_rect *rect); +int drm_fbconv_blit_fullscreen(void *dst, void *vaddr, + struct drm_framebuffer *fb); /* * Modeset helpers
The display mode is set by converting the DRM display mode to an fb_info state and handling it to the fbdev driver's fb_setvar() function. This also requires a color depth, which we take from the value of struct drm_mode_config.preferred_depth Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/drm_fbconv_helper.c | 113 +++++++++++++++++++++++++++- include/drm/drm_fbconv_helper.h | 2 + 2 files changed, 113 insertions(+), 2 deletions(-)