| Message ID | 1432132428-13519-1-git-send-email-gustavo@padovan.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 2015-05-20 16:33, Gustavo Padovan wrote: > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > When mode's vrefresh is zero we should ask DRM core to calculate > vrefresh > for us so we can get the correct value instead of relying on fixed > value > defined in a macro. > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > --- > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > index 9819fa6..08f7197 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > @@ -42,7 +42,6 @@ > * CPU Interface. > */ > > -#define FIMD_DEFAULT_FRAMERATE 60 > #define MIN_FB_WIDTH_FOR_16WORD_BURST 128 > > /* position control register for hardware window 0, 2 ~ 4.*/ > @@ -329,7 +328,7 @@ static bool fimd_mode_fixup(struct exynos_drm_crtc > *crtc, > struct drm_display_mode *adjusted_mode) > { > if (adjusted_mode->vrefresh == 0) Well, I'm not completly sure how this all works, but shouldn't we check 'mode' here, and not 'adjusted_mode'? With best wishes, Tobias > - adjusted_mode->vrefresh = FIMD_DEFAULT_FRAMERATE; > + adjusted_mode->vrefresh = drm_mode_vrefresh(mode); > > return true; > } -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 20 May 2015 at 17:58, Tobias Jakobi <tjakobi@math.uni-bielefeld.de> wrote: > On 2015-05-20 16:33, Gustavo Padovan wrote: >> When mode's vrefresh is zero we should ask DRM core to calculate vrefresh >> for us so we can get the correct value instead of relying on fixed value >> defined in a macro. >> >> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> >> --- >> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> index 9819fa6..08f7197 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> @@ -42,7 +42,6 @@ >> * CPU Interface. >> */ >> >> -#define FIMD_DEFAULT_FRAMERATE 60 >> #define MIN_FB_WIDTH_FOR_16WORD_BURST 128 >> >> /* position control register for hardware window 0, 2 ~ 4.*/ >> @@ -329,7 +328,7 @@ static bool fimd_mode_fixup(struct exynos_drm_crtc >> *crtc, >> struct drm_display_mode *adjusted_mode) >> { >> if (adjusted_mode->vrefresh == 0) > > Well, I'm not completly sure how this all works, but shouldn't we check > 'mode' here, and not 'adjusted_mode'? adjusted_mode is fine; it is pre-populated with the value from mode. 'mode' itself _must not_ be modified. Mind you, this is missing a hunk to reject entirely invalid modes, i.e.: if (adjusted_mode->vrefresh == 0) adjusted_mode->vrefresh = drm_mode_vrefresh(adjusted_mode); if (adjusted_mode->vrefresh == 0) return false; Cheers, Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, 2015-05-20 Daniel Stone <daniel@fooishbar.org>: > Hi, > > On 20 May 2015 at 17:58, Tobias Jakobi <tjakobi@math.uni-bielefeld.de> wrote: > > On 2015-05-20 16:33, Gustavo Padovan wrote: > >> When mode's vrefresh is zero we should ask DRM core to calculate vrefresh > >> for us so we can get the correct value instead of relying on fixed value > >> defined in a macro. > >> > >> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > >> --- > >> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 3 +-- > >> 1 file changed, 1 insertion(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > >> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > >> index 9819fa6..08f7197 100644 > >> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > >> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > >> @@ -42,7 +42,6 @@ > >> * CPU Interface. > >> */ > >> > >> -#define FIMD_DEFAULT_FRAMERATE 60 > >> #define MIN_FB_WIDTH_FOR_16WORD_BURST 128 > >> > >> /* position control register for hardware window 0, 2 ~ 4.*/ > >> @@ -329,7 +328,7 @@ static bool fimd_mode_fixup(struct exynos_drm_crtc > >> *crtc, > >> struct drm_display_mode *adjusted_mode) > >> { > >> if (adjusted_mode->vrefresh == 0) > > > > Well, I'm not completly sure how this all works, but shouldn't we check > > 'mode' here, and not 'adjusted_mode'? > > adjusted_mode is fine; it is pre-populated with the value from mode. > 'mode' itself _must not_ be modified. > > Mind you, this is missing a hunk to reject entirely invalid modes, i.e.: > if (adjusted_mode->vrefresh == 0) > adjusted_mode->vrefresh = drm_mode_vrefresh(adjusted_mode); > if (adjusted_mode->vrefresh == 0) > return false; I have sent a v2 with the proposed change. I can't really test this here as I don't have the hardware where it fails. So in v1 I was just guessing that it would fix the issue. Gustavo -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 9819fa6..08f7197 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -42,7 +42,6 @@ * CPU Interface. */ -#define FIMD_DEFAULT_FRAMERATE 60 #define MIN_FB_WIDTH_FOR_16WORD_BURST 128 /* position control register for hardware window 0, 2 ~ 4.*/ @@ -329,7 +328,7 @@ static bool fimd_mode_fixup(struct exynos_drm_crtc *crtc, struct drm_display_mode *adjusted_mode) { if (adjusted_mode->vrefresh == 0) - adjusted_mode->vrefresh = FIMD_DEFAULT_FRAMERATE; + adjusted_mode->vrefresh = drm_mode_vrefresh(mode); return true; }