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
signature.asc
Description: PGP signature
