On Fri, Feb 21, 2020 at 06:27:22PM +0100, Sam Ravnborg wrote:
Hi Ville.
On Wed, Feb 19, 2020 at 10:35:41PM +0200, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Store the timings (apart from the clock) as u16. The uapi mode struct already uses u16 for everything so using something bigger internally doesn't really help us.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
While touchign this - have you cinsidered to put all the timing fields in a dedicated struct - and then have two instanses of said struct here?
The normal vs. crtc timigns aren't the same. But I've been pondering about pulling the crtc timings into a separate struct and using that in some places where we currently use a full drm_display_mode but only really care about the crtc timings.
Like:
/**
- struct panel_timing - timing configuratio for a panel
- The timing is in khz and ...
/* struct panel_timing { /** * @clock: Panel clock in khz */ int clock;
/** * @hdisplay: bla bla */ u16 hdisplay; u16 hsync_start; u16 hsync_end; u16 htotal; u16 hskew; u16 vdisplay; u16 vsync_start; u16 vsync_end; u16 vtotal; u16 vscan; };
And then in drm_display_mode:
/** * @hw_timing: The timing for the panel */ struct panel_timing hw_timing;
/** * @calc_timing: The calculated timing */ struct panel_timing calc_timing;
Or something like that. We could use the panel_timign struct in some other places I think.
Sam
drivers/gpu/drm/drm_modes.c | 7 ------ include/drm/drm_modes.h | 46 ++++++++++++++++++------------------- 2 files changed, 23 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 0e7c9ba241c4..cc9fc52f9f7c 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1917,13 +1917,6 @@ EXPORT_SYMBOL(drm_mode_create_from_cmdline_mode); void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out, const struct drm_display_mode *in) {
- WARN(in->hdisplay > USHRT_MAX || in->hsync_start > USHRT_MAX ||
in->hsync_end > USHRT_MAX || in->htotal > USHRT_MAX ||in->hskew > USHRT_MAX || in->vdisplay > USHRT_MAX ||in->vsync_start > USHRT_MAX || in->vsync_end > USHRT_MAX ||in->vtotal > USHRT_MAX || in->vscan > USHRT_MAX,"timing values too large for mode info\n");- out->clock = in->clock; out->hdisplay = in->hdisplay; out->hsync_start = in->hsync_start;
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index b28c0234fcd7..b585074945b5 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -278,16 +278,16 @@ struct drm_display_mode { * Pixel clock in kHz. */ int clock; /* in kHz */
- int hdisplay;
- int hsync_start;
- int hsync_end;
- int htotal;
- int hskew;
- int vdisplay;
- int vsync_start;
- int vsync_end;
- int vtotal;
- int vscan;
- u16 hdisplay;
- u16 hsync_start;
- u16 hsync_end;
- u16 htotal;
- u16 hskew;
- u16 vdisplay;
- u16 vsync_start;
- u16 vsync_end;
- u16 vtotal;
- u16 vscan; /**
- @flags:
@@ -356,19 +356,19 @@ struct drm_display_mode { * difference is exactly a factor of 10. */ int crtc_clock;
- int crtc_hdisplay;
- int crtc_hblank_start;
- int crtc_hblank_end;
- int crtc_hsync_start;
- int crtc_hsync_end;
- int crtc_htotal;
- int crtc_hskew;
- int crtc_vdisplay;
- int crtc_vblank_start;
- int crtc_vblank_end;
- int crtc_vsync_start;
- int crtc_vsync_end;
- int crtc_vtotal;
u16 crtc_hdisplay;
u16 crtc_hblank_start;
u16 crtc_hblank_end;
u16 crtc_hsync_start;
u16 crtc_hsync_end;
u16 crtc_htotal;
u16 crtc_hskew;
u16 crtc_vdisplay;
u16 crtc_vblank_start;
u16 crtc_vblank_end;
u16 crtc_vsync_start;
u16 crtc_vsync_end;
u16 crtc_vtotal;
/**
- @private_flags:
-- 2.24.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel