| Message ID | 20240618081418.250953-2-j-choudhary@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SN65DSI86 minor fixes | expand |
On Tue, Jun 18, 2024 at 01:44:17PM GMT, Jayesh Choudhary wrote: > Add the atomic_check hook to ensure that the parameters are within the > valid range. > As of now, dsi clock freqency is being calculated in bridge_enable but > this needs to be checked in atomic_check which is called before > bridge_enable so move this calculation to atomic_check and write the > register value in bridge_enable as it is. > > For now, add mode clock check for the max resolution supported by the > bridge as mentioned in the SN65DSI86 datasheet[0] and dsi clock range > check for SN_DSIA_CLK_FREQ_REG. > According to the datasheet[0], the minimum value for that reg is 0x08 > and the maximum value is 0x96. So add check for that. > > [0]: <https://www.ti.com/lit/gpn/sn65dsi86> > > Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com> > --- > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 65 +++++++++++++++++++-------- > 1 file changed, 46 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > index 84698a0b27a8..d13b42d7c512 100644 > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > @@ -113,6 +113,20 @@ > > #define MIN_DSI_CLK_FREQ_MHZ 40 > > +/* > + * NOTE: DSI clock frequency range: [40MHz,755MHz) > + * DSI clock frequency range is in 5-MHz increments > + * So [40MHz,45MHz) translates to 0x08 (min value) > + * And [750MHz,755MHz) translates to 0x96 (max value) > + */ > +#define MIN_DSI_CLK_RANGE 0x8 > +#define MAX_DSI_CLK_RANGE 0x96 > + > +/* Pixel clock to support max resolution (4K@60Hz) supported > + * by the bridge. > + */ > +#define SN65DSI86_MAX_PIXEL_CLOCK_KHZ 600000 > + > /* fudge factor required to account for 8b/10b encoding */ > #define DP_CLK_FUDGE_NUM 10 > #define DP_CLK_FUDGE_DEN 8 > @@ -191,6 +205,7 @@ struct ti_sn65dsi86 { > u8 ln_polrs; > bool comms_enabled; > struct mutex comms_mutex; > + u32 dsi_clk_range; > > #if defined(CONFIG_OF_GPIO) > struct gpio_chip gchip; > @@ -820,24 +835,6 @@ static void ti_sn_bridge_atomic_disable(struct drm_bridge *bridge, > regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, VSTREAM_ENABLE, 0); > } > > -static void ti_sn_bridge_set_dsi_rate(struct ti_sn65dsi86 *pdata) > -{ > - unsigned int bit_rate_mhz, clk_freq_mhz; > - unsigned int val; > - struct drm_display_mode *mode = > - &pdata->bridge.encoder->crtc->state->adjusted_mode; > - > - /* set DSIA clk frequency */ > - bit_rate_mhz = (mode->clock / 1000) * > - mipi_dsi_pixel_format_to_bpp(pdata->dsi->format); > - clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes * 2); > - > - /* for each increment in val, frequency increases by 5MHz */ > - val = (MIN_DSI_CLK_FREQ_MHZ / 5) + > - (((clk_freq_mhz - MIN_DSI_CLK_FREQ_MHZ) / 5) & 0xFF); > - regmap_write(pdata->regmap, SN_DSIA_CLK_FREQ_REG, val); > -} > - > static unsigned int ti_sn_bridge_get_bpp(struct drm_connector *connector) > { > if (connector->display_info.bpc <= 6) > @@ -1104,7 +1101,7 @@ static void ti_sn_bridge_atomic_enable(struct drm_bridge *bridge, > pdata->ln_polrs << LN_POLRS_OFFSET); > > /* set dsi clk frequency value */ > - ti_sn_bridge_set_dsi_rate(pdata); > + regmap_write(pdata->regmap, SN_DSIA_CLK_FREQ_REG, pdata->dsi_clk_range); > > /* > * The SN65DSI86 only supports ASSR Display Authentication method and > @@ -1215,6 +1212,35 @@ static const struct drm_edid *ti_sn_bridge_edid_read(struct drm_bridge *bridge, > return drm_edid_read_ddc(connector, &pdata->aux.ddc); > } > > +static int ti_sn_bridge_atomic_check(struct drm_bridge *bridge, > + struct drm_bridge_state *bridge_state, > + struct drm_crtc_state *crtc_state, > + struct drm_connector_state *conn_state) > +{ > + struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge); > + struct drm_display_mode *mode = &crtc_state->mode; > + unsigned int bit_rate_mhz, clk_freq_mhz; > + > + /* Pixel clock check */ > + if (mode->clock > SN65DSI86_MAX_PIXEL_CLOCK_KHZ) > + return -EINVAL; > + > + bit_rate_mhz = (mode->clock / 1000) * > + mipi_dsi_pixel_format_to_bpp(pdata->dsi->format); > + clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes * 2); > + > + /* for each increment in dsi_clk_range, frequency increases by 5MHz */ > + pdata->dsi_clk_range = (MIN_DSI_CLK_FREQ_MHZ / 5) + > + (((clk_freq_mhz - MIN_DSI_CLK_FREQ_MHZ) / 5) & 0xFF); atomic_check might be called several times, it might be called to test the state. As such, it should not modify anything outside of the state variables. > + > + /* SN_DSIA_CLK_FREQ_REG check */ > + if (pdata->dsi_clk_range > MAX_DSI_CLK_RANGE || > + pdata->dsi_clk_range < MIN_DSI_CLK_RANGE) > + return -EINVAL; > + > + return 0; > +} > + > static const struct drm_bridge_funcs ti_sn_bridge_funcs = { > .attach = ti_sn_bridge_attach, > .detach = ti_sn_bridge_detach, > @@ -1228,6 +1254,7 @@ static const struct drm_bridge_funcs ti_sn_bridge_funcs = { > .atomic_reset = drm_atomic_helper_bridge_reset, > .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, > .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, > + .atomic_check = ti_sn_bridge_atomic_check, > }; > > static void ti_sn_bridge_parse_lanes(struct ti_sn65dsi86 *pdata, > -- > 2.25.1 >
Hello Dmitry, Thanks for the review. On 18/06/24 14:29, Dmitry Baryshkov wrote: > On Tue, Jun 18, 2024 at 01:44:17PM GMT, Jayesh Choudhary wrote: >> Add the atomic_check hook to ensure that the parameters are within the >> valid range. >> As of now, dsi clock freqency is being calculated in bridge_enable but >> this needs to be checked in atomic_check which is called before >> bridge_enable so move this calculation to atomic_check and write the >> register value in bridge_enable as it is. >> >> For now, add mode clock check for the max resolution supported by the >> bridge as mentioned in the SN65DSI86 datasheet[0] and dsi clock range >> check for SN_DSIA_CLK_FREQ_REG. >> According to the datasheet[0], the minimum value for that reg is 0x08 >> and the maximum value is 0x96. So add check for that. >> >> [0]: <https://www.ti.com/lit/gpn/sn65dsi86> >> >> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com> >> --- >> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 65 +++++++++++++++++++-------- >> 1 file changed, 46 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c >> index 84698a0b27a8..d13b42d7c512 100644 >> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c >> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c >> @@ -113,6 +113,20 @@ >> [...] >> >> +static int ti_sn_bridge_atomic_check(struct drm_bridge *bridge, >> + struct drm_bridge_state *bridge_state, >> + struct drm_crtc_state *crtc_state, >> + struct drm_connector_state *conn_state) >> +{ >> + struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge); >> + struct drm_display_mode *mode = &crtc_state->mode; >> + unsigned int bit_rate_mhz, clk_freq_mhz; >> + >> + /* Pixel clock check */ >> + if (mode->clock > SN65DSI86_MAX_PIXEL_CLOCK_KHZ) >> + return -EINVAL; >> + >> + bit_rate_mhz = (mode->clock / 1000) * >> + mipi_dsi_pixel_format_to_bpp(pdata->dsi->format); >> + clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes * 2); >> + >> + /* for each increment in dsi_clk_range, frequency increases by 5MHz */ >> + pdata->dsi_clk_range = (MIN_DSI_CLK_FREQ_MHZ / 5) + >> + (((clk_freq_mhz - MIN_DSI_CLK_FREQ_MHZ) / 5) & 0xFF); > > atomic_check might be called several times, it might be called to test > the state. As such, it should not modify anything outside of the > state variables. > If not in atomic_check, then where should I move this calculation and check? mode_valid with returning MODE_BAD in case of failure? I had to move it from bridge_enable based on the comments on v1: https://patchwork.kernel.org/project/dri-devel/patch/20240408073623.186489-1-j-choudhary@ti.com/#25801801 Warm Regards, Jayesh [...]
On Tue, 18 Jun 2024 at 12:56, Jayesh Choudhary <j-choudhary@ti.com> wrote: > > Hello Dmitry, > > Thanks for the review. > > On 18/06/24 14:29, Dmitry Baryshkov wrote: > > On Tue, Jun 18, 2024 at 01:44:17PM GMT, Jayesh Choudhary wrote: > >> Add the atomic_check hook to ensure that the parameters are within the > >> valid range. > >> As of now, dsi clock freqency is being calculated in bridge_enable but > >> this needs to be checked in atomic_check which is called before > >> bridge_enable so move this calculation to atomic_check and write the > >> register value in bridge_enable as it is. > >> > >> For now, add mode clock check for the max resolution supported by the > >> bridge as mentioned in the SN65DSI86 datasheet[0] and dsi clock range > >> check for SN_DSIA_CLK_FREQ_REG. > >> According to the datasheet[0], the minimum value for that reg is 0x08 > >> and the maximum value is 0x96. So add check for that. > >> > >> [0]: <https://www.ti.com/lit/gpn/sn65dsi86> > >> > >> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com> > >> --- > >> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 65 +++++++++++++++++++-------- > >> 1 file changed, 46 insertions(+), 19 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > >> index 84698a0b27a8..d13b42d7c512 100644 > >> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > >> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > >> @@ -113,6 +113,20 @@ > >> > > [...] > > >> > >> +static int ti_sn_bridge_atomic_check(struct drm_bridge *bridge, > >> + struct drm_bridge_state *bridge_state, > >> + struct drm_crtc_state *crtc_state, > >> + struct drm_connector_state *conn_state) > >> +{ > >> + struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge); > >> + struct drm_display_mode *mode = &crtc_state->mode; > >> + unsigned int bit_rate_mhz, clk_freq_mhz; > >> + > >> + /* Pixel clock check */ > >> + if (mode->clock > SN65DSI86_MAX_PIXEL_CLOCK_KHZ) > >> + return -EINVAL; > >> + > >> + bit_rate_mhz = (mode->clock / 1000) * > >> + mipi_dsi_pixel_format_to_bpp(pdata->dsi->format); > >> + clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes * 2); > >> + > >> + /* for each increment in dsi_clk_range, frequency increases by 5MHz */ > >> + pdata->dsi_clk_range = (MIN_DSI_CLK_FREQ_MHZ / 5) + > >> + (((clk_freq_mhz - MIN_DSI_CLK_FREQ_MHZ) / 5) & 0xFF); > > > > atomic_check might be called several times, it might be called to test > > the state. As such, it should not modify anything outside of the > > state variables. > > > > If not in atomic_check, then where should I move this calculation and check? > mode_valid with returning MODE_BAD in case of failure? I didn't write that it's the wrong place for math. I wrote that you should not be modifying global structure. So you have to subclass drm_bridge_state for the driver and store the value there. Or just add a helper function and call it from atomic_check(), mode_valid() and set_dsi_rate(). It really looks like a simpler solution here. Note, there is a significant difference between mode_valid() and atomic_check(). The former function is used for filtering the modes, while the latter one is used for actually checking that the parameters passed from the client are correct. > > I had to move it from bridge_enable based on the comments on v1: > https://patchwork.kernel.org/project/dri-devel/patch/20240408073623.186489-1-j-choudhary@ti.com/#25801801 > > Warm Regards, > Jayesh > > [...]
Hello Dmitry, On 18/06/24 15:45, Dmitry Baryshkov wrote: > On Tue, 18 Jun 2024 at 12:56, Jayesh Choudhary <j-choudhary@ti.com> wrote: >> >> Hello Dmitry, >> >> Thanks for the review. >> >> On 18/06/24 14:29, Dmitry Baryshkov wrote: >>> On Tue, Jun 18, 2024 at 01:44:17PM GMT, Jayesh Choudhary wrote: >>>> Add the atomic_check hook to ensure that the parameters are within the >>>> valid range. >>>> As of now, dsi clock freqency is being calculated in bridge_enable but >>>> this needs to be checked in atomic_check which is called before >>>> bridge_enable so move this calculation to atomic_check and write the >>>> register value in bridge_enable as it is. >>>> >>>> For now, add mode clock check for the max resolution supported by the >>>> bridge as mentioned in the SN65DSI86 datasheet[0] and dsi clock range >>>> check for SN_DSIA_CLK_FREQ_REG. >>>> According to the datasheet[0], the minimum value for that reg is 0x08 >>>> and the maximum value is 0x96. So add check for that. >>>> >>>> [0]: <https://www.ti.com/lit/gpn/sn65dsi86> >>>> >>>> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com> >>>> --- >>>> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 65 +++++++++++++++++++-------- >>>> 1 file changed, 46 insertions(+), 19 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c >>>> index 84698a0b27a8..d13b42d7c512 100644 >>>> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c >>>> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c >>>> @@ -113,6 +113,20 @@ >>>> >> >> [...] >> >>>> >>>> +static int ti_sn_bridge_atomic_check(struct drm_bridge *bridge, >>>> + struct drm_bridge_state *bridge_state, >>>> + struct drm_crtc_state *crtc_state, >>>> + struct drm_connector_state *conn_state) >>>> +{ >>>> + struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge); >>>> + struct drm_display_mode *mode = &crtc_state->mode; >>>> + unsigned int bit_rate_mhz, clk_freq_mhz; >>>> + >>>> + /* Pixel clock check */ >>>> + if (mode->clock > SN65DSI86_MAX_PIXEL_CLOCK_KHZ) >>>> + return -EINVAL; >>>> + >>>> + bit_rate_mhz = (mode->clock / 1000) * >>>> + mipi_dsi_pixel_format_to_bpp(pdata->dsi->format); >>>> + clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes * 2); >>>> + >>>> + /* for each increment in dsi_clk_range, frequency increases by 5MHz */ >>>> + pdata->dsi_clk_range = (MIN_DSI_CLK_FREQ_MHZ / 5) + >>>> + (((clk_freq_mhz - MIN_DSI_CLK_FREQ_MHZ) / 5) & 0xFF); >>> >>> atomic_check might be called several times, it might be called to test >>> the state. As such, it should not modify anything outside of the >>> state variables. >>> >> >> If not in atomic_check, then where should I move this calculation and check? >> mode_valid with returning MODE_BAD in case of failure? > > I didn't write that it's the wrong place for math. I wrote that you > should not be modifying global structure. > > So you have to subclass drm_bridge_state for the driver and store the > value there. Or just add a helper function and call it from > atomic_check(), mode_valid() and set_dsi_rate(). It really looks like > a simpler solution here. > Okay, instead of moving the set_dsi_rate, I will rename it to calc_dsi_rate with integer return value which I would use in both bridge enable to write the register value and atomic_check to check the parameters eliminating the need to modify the pdata structure/ adding new variable to the structure. (Earlier I was trying to avoid calculation in both calls so I added another variable to the structure and used that. But I get your point now!) I will re-order the patches to have an independent fix patch addressing your concern in [2/2] patch https://lore.kernel.org/all/CAA8EJpq2UkMn9ArSNaJcOyw28H4uUcRwvUqfUBBqSCALmozBrg@mail.gmail.com/ Also in the code they have been using 594MHz as mode clock limit. I was using more relaxed value (600MHz) in atomic check but I will switch to 594MHz to be in sync with the value that is used in the driver. > Note, there is a significant difference between mode_valid() and > atomic_check(). The former function is used for filtering the modes, > while the latter one is used for actually checking that the parameters > passed from the client are correct. [...] Warm Regards, Jayesh
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c index 84698a0b27a8..d13b42d7c512 100644 --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c @@ -113,6 +113,20 @@ #define MIN_DSI_CLK_FREQ_MHZ 40 +/* + * NOTE: DSI clock frequency range: [40MHz,755MHz) + * DSI clock frequency range is in 5-MHz increments + * So [40MHz,45MHz) translates to 0x08 (min value) + * And [750MHz,755MHz) translates to 0x96 (max value) + */ +#define MIN_DSI_CLK_RANGE 0x8 +#define MAX_DSI_CLK_RANGE 0x96 + +/* Pixel clock to support max resolution (4K@60Hz) supported + * by the bridge. + */ +#define SN65DSI86_MAX_PIXEL_CLOCK_KHZ 600000 + /* fudge factor required to account for 8b/10b encoding */ #define DP_CLK_FUDGE_NUM 10 #define DP_CLK_FUDGE_DEN 8 @@ -191,6 +205,7 @@ struct ti_sn65dsi86 { u8 ln_polrs; bool comms_enabled; struct mutex comms_mutex; + u32 dsi_clk_range; #if defined(CONFIG_OF_GPIO) struct gpio_chip gchip; @@ -820,24 +835,6 @@ static void ti_sn_bridge_atomic_disable(struct drm_bridge *bridge, regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, VSTREAM_ENABLE, 0); } -static void ti_sn_bridge_set_dsi_rate(struct ti_sn65dsi86 *pdata) -{ - unsigned int bit_rate_mhz, clk_freq_mhz; - unsigned int val; - struct drm_display_mode *mode = - &pdata->bridge.encoder->crtc->state->adjusted_mode; - - /* set DSIA clk frequency */ - bit_rate_mhz = (mode->clock / 1000) * - mipi_dsi_pixel_format_to_bpp(pdata->dsi->format); - clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes * 2); - - /* for each increment in val, frequency increases by 5MHz */ - val = (MIN_DSI_CLK_FREQ_MHZ / 5) + - (((clk_freq_mhz - MIN_DSI_CLK_FREQ_MHZ) / 5) & 0xFF); - regmap_write(pdata->regmap, SN_DSIA_CLK_FREQ_REG, val); -} - static unsigned int ti_sn_bridge_get_bpp(struct drm_connector *connector) { if (connector->display_info.bpc <= 6) @@ -1104,7 +1101,7 @@ static void ti_sn_bridge_atomic_enable(struct drm_bridge *bridge, pdata->ln_polrs << LN_POLRS_OFFSET); /* set dsi clk frequency value */ - ti_sn_bridge_set_dsi_rate(pdata); + regmap_write(pdata->regmap, SN_DSIA_CLK_FREQ_REG, pdata->dsi_clk_range); /* * The SN65DSI86 only supports ASSR Display Authentication method and @@ -1215,6 +1212,35 @@ static const struct drm_edid *ti_sn_bridge_edid_read(struct drm_bridge *bridge, return drm_edid_read_ddc(connector, &pdata->aux.ddc); } +static int ti_sn_bridge_atomic_check(struct drm_bridge *bridge, + struct drm_bridge_state *bridge_state, + struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state) +{ + struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge); + struct drm_display_mode *mode = &crtc_state->mode; + unsigned int bit_rate_mhz, clk_freq_mhz; + + /* Pixel clock check */ + if (mode->clock > SN65DSI86_MAX_PIXEL_CLOCK_KHZ) + return -EINVAL; + + bit_rate_mhz = (mode->clock / 1000) * + mipi_dsi_pixel_format_to_bpp(pdata->dsi->format); + clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes * 2); + + /* for each increment in dsi_clk_range, frequency increases by 5MHz */ + pdata->dsi_clk_range = (MIN_DSI_CLK_FREQ_MHZ / 5) + + (((clk_freq_mhz - MIN_DSI_CLK_FREQ_MHZ) / 5) & 0xFF); + + /* SN_DSIA_CLK_FREQ_REG check */ + if (pdata->dsi_clk_range > MAX_DSI_CLK_RANGE || + pdata->dsi_clk_range < MIN_DSI_CLK_RANGE) + return -EINVAL; + + return 0; +} + static const struct drm_bridge_funcs ti_sn_bridge_funcs = { .attach = ti_sn_bridge_attach, .detach = ti_sn_bridge_detach, @@ -1228,6 +1254,7 @@ static const struct drm_bridge_funcs ti_sn_bridge_funcs = { .atomic_reset = drm_atomic_helper_bridge_reset, .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, + .atomic_check = ti_sn_bridge_atomic_check, }; static void ti_sn_bridge_parse_lanes(struct ti_sn65dsi86 *pdata,
Add the atomic_check hook to ensure that the parameters are within the valid range. As of now, dsi clock freqency is being calculated in bridge_enable but this needs to be checked in atomic_check which is called before bridge_enable so move this calculation to atomic_check and write the register value in bridge_enable as it is. For now, add mode clock check for the max resolution supported by the bridge as mentioned in the SN65DSI86 datasheet[0] and dsi clock range check for SN_DSIA_CLK_FREQ_REG. According to the datasheet[0], the minimum value for that reg is 0x08 and the maximum value is 0x96. So add check for that. [0]: <https://www.ti.com/lit/gpn/sn65dsi86> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com> --- drivers/gpu/drm/bridge/ti-sn65dsi86.c | 65 +++++++++++++++++++-------- 1 file changed, 46 insertions(+), 19 deletions(-)