| Message ID | 20230801101030.2040-1-keith.zhao@starfivetech.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | DRM driver for verisilicon | expand |
Hi, On Tue, Aug 01, 2023 at 06:10:29PM +0800, Keith Zhao wrote: > +static int vs_crtc_atomic_set_property(struct drm_crtc *crtc, > + struct drm_crtc_state *state, > + struct drm_property *property, > + uint64_t val) > +{ > + struct vs_crtc *vs_crtc = to_vs_crtc(crtc); > + struct vs_crtc_state *vs_crtc_state = to_vs_crtc_state(state); > + > + if (property == vs_crtc->sync_mode) > + vs_crtc_state->sync_mode = val; > + else if (property == vs_crtc->mmu_prefetch) > + vs_crtc_state->mmu_prefetch = val; > + else if (property == vs_crtc->bg_color) > + vs_crtc_state->bg_color = val; > + else if (property == vs_crtc->panel_sync) > + vs_crtc_state->sync_enable = val; > + else if (property == vs_crtc->dither) > + vs_crtc_state->dither_enable = val; > + else > + return -EINVAL; > + > + return 0; > +} > + > +static int vs_crtc_atomic_get_property(struct drm_crtc *crtc, > + const struct drm_crtc_state *state, > + struct drm_property *property, > + uint64_t *val) > +{ > + struct vs_crtc *vs_crtc = to_vs_crtc(crtc); > + const struct vs_crtc_state *vs_crtc_state = > + container_of(state, const struct vs_crtc_state, base); > + > + if (property == vs_crtc->sync_mode) > + *val = vs_crtc_state->sync_mode; > + else if (property == vs_crtc->mmu_prefetch) > + *val = vs_crtc_state->mmu_prefetch; > + else if (property == vs_crtc->bg_color) > + *val = vs_crtc_state->bg_color; > + else if (property == vs_crtc->panel_sync) > + *val = vs_crtc_state->sync_enable; > + else if (property == vs_crtc->dither) > + *val = vs_crtc_state->dither_enable; > + else > + return -EINVAL; > + > + return 0; > +} Any new property needs to follow these requirements: https://docs.kernel.org/gpu/drm-kms.html#requirements https://docs.kernel.org/gpu/drm-uapi.html#open-source-userspace-requirements Also, most of them are suspicious, like sync_mode, mmu_prefetch, panel_sync or dither_enable. Why would you want userspace to change those ? > +static int vs_crtc_late_register(struct drm_crtc *crtc) > +{ > + return 0; > +} You can drop that. > +static int vs_crtc_enable_vblank(struct drm_crtc *crtc) > +{ > + struct vs_crtc *vs_crtc = to_vs_crtc(crtc); > + > + vs_dc_enable_vblank(vs_crtc->dev, true); > + > + return 0; > +} > + > +static void vs_crtc_disable_vblank(struct drm_crtc *crtc) > +{ > + struct vs_crtc *vs_crtc = to_vs_crtc(crtc); > + > + vs_dc_enable_vblank(vs_crtc->dev, false); > +} > + > +static const struct drm_crtc_funcs vs_crtc_funcs = { > + .set_config = drm_atomic_helper_set_config, > + .page_flip = drm_atomic_helper_page_flip, > + .reset = vs_crtc_reset, > + .atomic_duplicate_state = vs_crtc_atomic_duplicate_state, > + .atomic_destroy_state = vs_crtc_atomic_destroy_state, > + .atomic_set_property = vs_crtc_atomic_set_property, > + .atomic_get_property = vs_crtc_atomic_get_property, > + .late_register = vs_crtc_late_register, > + .enable_vblank = vs_crtc_enable_vblank, > + .disable_vblank = vs_crtc_disable_vblank, > +}; > + > +static u8 cal_pixel_bits(u32 bus_format) > +{ > + u8 bpp; > + > + switch (bus_format) { > + case MEDIA_BUS_FMT_RGB565_1X16: > + case MEDIA_BUS_FMT_UYVY8_1X16: > + bpp = 16; > + break; > + case MEDIA_BUS_FMT_RGB666_1X18: > + case MEDIA_BUS_FMT_RGB666_1X24_CPADHI: > + bpp = 18; > + break; > + case MEDIA_BUS_FMT_UYVY10_1X20: > + bpp = 20; > + break; > + case MEDIA_BUS_FMT_BGR888_1X24: > + case MEDIA_BUS_FMT_UYYVYY8_0_5X24: > + case MEDIA_BUS_FMT_YUV8_1X24: > + bpp = 24; > + break; > + case MEDIA_BUS_FMT_RGB101010_1X30: > + case MEDIA_BUS_FMT_UYYVYY10_0_5X30: > + case MEDIA_BUS_FMT_YUV10_1X30: > + bpp = 30; > + break; > + default: > + bpp = 24; > + break; > + } > + > + return bpp; > +} > + > +static bool vs_crtc_mode_fixup(struct drm_crtc *crtc, > + const struct drm_display_mode *mode, > + struct drm_display_mode *adjusted_mode) > +{ > + struct vs_crtc *vs_crtc = to_vs_crtc(crtc); > + > + return vs_dc_mode_fixup(vs_crtc->dev, mode, adjusted_mode); > +} You should be using atomic_check. Maxime