| Message ID | 1507801165-21285-1-git-send-email-jsarha@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 12, 2017 at 12:39:25PM +0300, Jyri Sarha wrote: > We are using the vrefresh to check if we are too close to vertical > sync to update the two framebuffer DMA registers and risk a > collision. The vrefresh is coming from user space and normally it is > not used for anything. For instance xserver leaves vrefresh to zero > causing a division by zero when setting the mode. We want to make sure > the value is valid and force its recalculation in > tilcdc_crtc_set_mode(). > > Reported-by: Kevin Hao <kexin.hao@windriver.com> > Fixes: 11abbc9f39e0 ("drm/tilcdc: Set framebuffer DMA address to HW only if CRTC is enabled") > Cc: <stable@vger.kernel.org> # v4.11+ > Signed-off-by: Jyri Sarha <jsarha@ti.com> > --- > drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > index 704db24..aa5e87c 100644 > --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > @@ -459,6 +459,16 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc) > drm_framebuffer_get(fb); > > crtc->hwmode = crtc->state->adjusted_mode; > + > + /* We are using the vrefresh to check if we are too close to > + * vertical sync to update the two framebuffer DMA registers > + * and risk a collision. The vrefresh is coming from user > + * space and normally it is not used for anything. We want to > + * make sure the value is valid and force its recalculation > + * here. > + */ > + crtc->hwmode.vrefresh = 0; Why do we need this? Thanks, Kevin > + crtc->hwmode.vrefresh = drm_mode_vrefresh(&crtc->hwmode); > } > > static void tilcdc_crtc_enable(struct drm_crtc *crtc) > -- > 1.9.1 > > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > >
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 10/12/17 12:53, Kevin Hao wrote: > On Thu, Oct 12, 2017 at 12:39:25PM +0300, Jyri Sarha wrote: >> We are using the vrefresh to check if we are too close to vertical >> sync to update the two framebuffer DMA registers and risk a >> collision. The vrefresh is coming from user space and normally it is >> not used for anything. For instance xserver leaves vrefresh to zero >> causing a division by zero when setting the mode. We want to make sure >> the value is valid and force its recalculation in >> tilcdc_crtc_set_mode(). >> >> Reported-by: Kevin Hao <kexin.hao@windriver.com> >> Fixes: 11abbc9f39e0 ("drm/tilcdc: Set framebuffer DMA address to HW only if CRTC is enabled") >> Cc: <stable@vger.kernel.org> # v4.11+ >> Signed-off-by: Jyri Sarha <jsarha@ti.com> >> --- >> drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c >> index 704db24..aa5e87c 100644 >> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c >> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c >> @@ -459,6 +459,16 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc) >> drm_framebuffer_get(fb); >> >> crtc->hwmode = crtc->state->adjusted_mode; >> + >> + /* We are using the vrefresh to check if we are too close to >> + * vertical sync to update the two framebuffer DMA registers >> + * and risk a collision. The vrefresh is coming from user >> + * space and normally it is not used for anything. We want to >> + * make sure the value is valid and force its recalculation >> + * here. >> + */ >> + crtc->hwmode.vrefresh = 0; > > Why do we need this? > The drm_mode_vrefresh() [1] checks if the vrefresh is > 0 and only calculates it if it is not. I want to make sure the value is correct by recalculating it always. This is to prevent buggy user space SW from causing undefined behaviour. Best regards, Jyri [1] int drm_mode_vrefresh(const struct drm_display_mode *mode) { int refresh = 0; unsigned int calc_val; if (mode->vrefresh > 0) refresh = mode->vrefresh; else if (mode->htotal > 0 && mode->vtotal > 0) { int vtotal; vtotal = mode->vtotal; /* work out vrefresh the value will be x1000 */ calc_val = (mode->clock * 1000); calc_val /= mode->htotal; refresh = (calc_val + vtotal / 2) / vtotal; if (mode->flags & DRM_MODE_FLAG_INTERLACE) refresh *= 2; if (mode->flags & DRM_MODE_FLAG_DBLSCAN) refresh /= 2; if (mode->vscan > 1) refresh /= mode->vscan; } return refresh; }
There is a new patch to replace this one: https://lists.freedesktop.org/archives/dri-devel/2017-October/154589.html Best regards, Jyri Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 10/12/17 12:39, Jyri Sarha wrote: > We are using the vrefresh to check if we are too close to vertical > sync to update the two framebuffer DMA registers and risk a > collision. The vrefresh is coming from user space and normally it is > not used for anything. For instance xserver leaves vrefresh to zero > causing a division by zero when setting the mode. We want to make sure > the value is valid and force its recalculation in > tilcdc_crtc_set_mode(). > > Reported-by: Kevin Hao <kexin.hao@windriver.com> > Fixes: 11abbc9f39e0 ("drm/tilcdc: Set framebuffer DMA address to HW only if CRTC is enabled") > Cc: <stable@vger.kernel.org> # v4.11+ > Signed-off-by: Jyri Sarha <jsarha@ti.com> > --- > drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > index 704db24..aa5e87c 100644 > --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > @@ -459,6 +459,16 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc) > drm_framebuffer_get(fb); > > crtc->hwmode = crtc->state->adjusted_mode; > + > + /* We are using the vrefresh to check if we are too close to > + * vertical sync to update the two framebuffer DMA registers > + * and risk a collision. The vrefresh is coming from user > + * space and normally it is not used for anything. We want to > + * make sure the value is valid and force its recalculation > + * here. > + */ > + crtc->hwmode.vrefresh = 0; > + crtc->hwmode.vrefresh = drm_mode_vrefresh(&crtc->hwmode); > } > > static void tilcdc_crtc_enable(struct drm_crtc *crtc) >
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index 704db24..aa5e87c 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -459,6 +459,16 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc) drm_framebuffer_get(fb); crtc->hwmode = crtc->state->adjusted_mode; + + /* We are using the vrefresh to check if we are too close to + * vertical sync to update the two framebuffer DMA registers + * and risk a collision. The vrefresh is coming from user + * space and normally it is not used for anything. We want to + * make sure the value is valid and force its recalculation + * here. + */ + crtc->hwmode.vrefresh = 0; + crtc->hwmode.vrefresh = drm_mode_vrefresh(&crtc->hwmode); } static void tilcdc_crtc_enable(struct drm_crtc *crtc)
We are using the vrefresh to check if we are too close to vertical sync to update the two framebuffer DMA registers and risk a collision. The vrefresh is coming from user space and normally it is not used for anything. For instance xserver leaves vrefresh to zero causing a division by zero when setting the mode. We want to make sure the value is valid and force its recalculation in tilcdc_crtc_set_mode(). Reported-by: Kevin Hao <kexin.hao@windriver.com> Fixes: 11abbc9f39e0 ("drm/tilcdc: Set framebuffer DMA address to HW only if CRTC is enabled") Cc: <stable@vger.kernel.org> # v4.11+ Signed-off-by: Jyri Sarha <jsarha@ti.com> --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 10 ++++++++++ 1 file changed, 10 insertions(+)