| Message ID | 1453464732-17241-3-git-send-email-lionel.g.landwerlin@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jan 22, 2016 at 12:12:08PM +0000, Lionel Landwerlin wrote: > v2: Register LUT size properties as range > > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Probably should have noticed/commented on this on your previous iteration, but should we also restrict these new properties to be atomic-only? I thought there was a consensus a while back that new functionality would only be exposed for atomic-ready userspace. That would mean also adding DRM_MODE_PROP_ATOMIC to your flags at creation. Sorry for not noticing this when I commented before! Also, you probably want to add a Cc: dri-devel to this patch since it deals with the DRM core and isn't i915-specific. Matt > --- > Documentation/DocBook/gpu.tmpl | 50 +++++++++++++++++- > drivers/gpu/drm/drm_atomic.c | 102 +++++++++++++++++++++++++++++++++++- > drivers/gpu/drm/drm_atomic_helper.c | 10 ++++ > drivers/gpu/drm/drm_crtc.c | 35 +++++++++++++ > drivers/gpu/drm/drm_crtc_helper.c | 33 ++++++++++++ > include/drm/drm_crtc.h | 43 ++++++++++++++- > include/drm/drm_crtc_helper.h | 3 ++ > include/uapi/drm/drm_mode.h | 15 ++++++ > 8 files changed, 287 insertions(+), 4 deletions(-) > > diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl > index 351e801..c17ac29 100644 > --- a/Documentation/DocBook/gpu.tmpl > +++ b/Documentation/DocBook/gpu.tmpl > @@ -2068,7 +2068,7 @@ void intel_crt_init(struct drm_device *dev) > <td valign="top" >property to suggest an Y offset for a connector</td> > </tr> > <tr> > - <td rowspan="3" valign="top" >Optional</td> > + <td rowspan="8" valign="top" >Optional</td> > <td valign="top" >“scaling mode”</td> > <td valign="top" >ENUM</td> > <td valign="top" >{ "None", "Full", "Center", "Full aspect" }</td> > @@ -2092,6 +2092,54 @@ void intel_crt_init(struct drm_device *dev) > <td valign="top" >TBD</td> > </tr> > <tr> > + <td valign="top" >“DEGAMMA_LUT”</td> > + <td valign="top" >BLOB</td> > + <td valign="top" >0</td> > + <td valign="top" >CRTC</td> > + <td valign="top" >DRM property to set the degamma LUT mapping > + pixel data from the framebuffer before it is given to the > + transformation matrix. The data is an interpreted as an array > + of struct drm_color_lut elements.</td> > + </tr> > + <tr> > + <td valign="top" >“DEGAMMA_LUT_SIZE”</td> > + <td valign="top" >RANGE | IMMUTABLE</td> > + <td valign="top" >Min=0, Max=UINT_MAX</td> > + <td valign="top" >CRTC</td> > + <td valign="top" >DRM property to gives the size of the LUT to > + be set on the DEGAMMA_LUT property (the size depends on the > + underlying hardware).</td> > + </tr> > + <tr> > + <td valign="top" >“CTM_MATRIX”</td> > + <td valign="top" >BLOB</td> > + <td valign="top" >0</td> > + <td valign="top" >CRTC</td> > + <td valign="top" >DRM property to set the transformation > + matrix apply to pixel data after the lookup through the > + degamma LUT and before the lookup through the gamma LUT. The > + data is an interpreted as a struct drm_color_ctm.</td> > + </tr> > + <tr> > + <td valign="top" >“GAMMA_LUT”</td> > + <td valign="top" >BLOB</td> > + <td valign="top" >0</td> > + <td valign="top" >CRTC</td> > + <td valign="top" >DRM property to set the gamma LUT mapping > + pixel data after to the transformation matrix to data sent to > + the connector. The data is an interpreted as an array > + of struct drm_color_lut elements.</td> > + </tr> > + <tr> > + <td valign="top" >“GAMMA_LUT_SIZE”</td> > + <td valign="top" >RANGE | IMMUTABLE</td> > + <td valign="top" >Min=0, Max=UINT_MAX</td> > + <td valign="top" >CRTC</td> > + <td valign="top" >DRM property to gives the size of the LUT to > + be set on the GAMMA_LUT property (the size depends on the > + underlying hardware).</td> > + </tr> > + <tr> > <td rowspan="20" valign="top" >i915</td> > <td rowspan="2" valign="top" >Generic</td> > <td valign="top" >"Broadcast RGB"</td> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 3f74193..5287416 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -28,6 +28,7 @@ > > #include <drm/drmP.h> > #include <drm/drm_atomic.h> > +#include <drm/drm_mode.h> > #include <drm/drm_plane_helper.h> > > /** > @@ -388,6 +389,58 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state, > EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc); > > /** > + * drm_atomic_replace_property_blob - replace a blob property > + * @blob: a pointer to the member blob to be replaced > + * @new_blob: the new blob to replace with > + * @expected_size: the expected size of the new blob > + * @replaced: whether the blob has been replaced > + * > + * RETURNS: > + * Zero on success, error code on failure > + */ > +static int > +drm_atomic_replace_property_blob(struct drm_property_blob **blob, > + struct drm_property_blob *new_blob, > + size_t expected_size, > + bool *replaced) > +{ > + struct drm_property_blob *old_blob = *blob; > + > + if (old_blob == new_blob) > + return 0; > + > + if (new_blob != NULL && new_blob->length != expected_size) > + return -EINVAL; > + > + if (old_blob != NULL) > + drm_property_unreference_blob(old_blob); > + *blob = new_blob; > + *replaced = true; > + > + return 0; > +} > + > +static int > +drm_atomic_replace_property_blob_from_id(struct drm_crtc *crtc, > + struct drm_property_blob **blob, > + uint64_t blob_id, > + size_t expected_size, > + bool *replaced) > +{ > + struct drm_device *dev = crtc->dev; > + struct drm_property_blob *new_blob = NULL; > + > + if (blob_id != 0) { > + new_blob = drm_property_lookup_blob(dev, blob_id); > + if (new_blob == NULL) > + return -EINVAL; > + } > + > + return drm_atomic_replace_property_blob(blob, new_blob, > + expected_size, replaced); > +} > + > +/** > * drm_atomic_crtc_set_property - set property on CRTC > * @crtc: the drm CRTC to set a property on > * @state: the state object to update with the new property value > @@ -419,8 +472,47 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc, > ret = drm_atomic_set_mode_prop_for_crtc(state, mode); > drm_property_unreference_blob(mode); > return ret; > - } > - else if (crtc->funcs->atomic_set_property) > + } else if (property == config->degamma_lut_property) { > + bool replaced = false; > + uint64_t lut_size = 0; > + > + ret = drm_object_property_get_value(&crtc->base, > + config->degamma_lut_size_property, > + &lut_size); > + if (ret == 0) > + ret = drm_atomic_replace_property_blob_from_id(crtc, > + &state->degamma_lut, > + val, > + lut_size * sizeof(struct drm_color_lut), > + &replaced); > + state->color_mgmt_changed = replaced; > + return ret; > + } else if (property == config->ctm_matrix_property) { > + bool replaced = false; > + > + ret = drm_atomic_replace_property_blob_from_id(crtc, > + &state->ctm_matrix, > + val, > + sizeof(struct drm_color_ctm), > + &replaced); > + state->color_mgmt_changed = replaced; > + return ret; > + } else if (property == config->gamma_lut_property) { > + bool replaced = false; > + uint64_t lut_size = 0; > + > + ret = drm_object_property_get_value(&crtc->base, > + config->gamma_lut_size_property, > + &lut_size); > + if (ret == 0) > + ret = drm_atomic_replace_property_blob_from_id(crtc, > + &state->gamma_lut, > + val, > + lut_size * sizeof(struct drm_color_lut), > + &replaced); > + state->color_mgmt_changed = replaced; > + return ret; > + } else if (crtc->funcs->atomic_set_property) > return crtc->funcs->atomic_set_property(crtc, state, property, val); > else > return -EINVAL; > @@ -456,6 +548,12 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc, > *val = state->active; > else if (property == config->prop_mode_id) > *val = (state->mode_blob) ? state->mode_blob->base.id : 0; > + else if (property == config->degamma_lut_property) > + *val = (state->degamma_lut) ? state->degamma_lut->base.id : 0; > + else if (property == config->ctm_matrix_property) > + *val = (state->ctm_matrix) ? state->ctm_matrix->base.id : 0; > + else if (property == config->gamma_lut_property) > + *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0; > else if (crtc->funcs->atomic_get_property) > return crtc->funcs->atomic_get_property(crtc, state, property, val); > else > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index f0c3984..e4a5755 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2454,10 +2454,17 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc, > > if (state->mode_blob) > drm_property_reference_blob(state->mode_blob); > + if (state->degamma_lut) > + drm_property_reference_blob(state->degamma_lut); > + if (state->ctm_matrix) > + drm_property_reference_blob(state->ctm_matrix); > + if (state->gamma_lut) > + drm_property_reference_blob(state->gamma_lut); > state->mode_changed = false; > state->active_changed = false; > state->planes_changed = false; > state->connectors_changed = false; > + state->color_mgmt_changed = false; > state->event = NULL; > } > EXPORT_SYMBOL(__drm_atomic_helper_crtc_duplicate_state); > @@ -2498,6 +2505,9 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc, > struct drm_crtc_state *state) > { > drm_property_unreference_blob(state->mode_blob); > + drm_property_unreference_blob(state->degamma_lut); > + drm_property_unreference_blob(state->ctm_matrix); > + drm_property_unreference_blob(state->gamma_lut); > } > EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state); > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index d40bab2..b1aa38a 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -1542,6 +1542,41 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) > return -ENOMEM; > dev->mode_config.prop_mode_id = prop; > > + prop = drm_property_create(dev, > + DRM_MODE_PROP_BLOB, > + "DEGAMMA_LUT", 0); > + if (!prop) > + return -ENOMEM; > + dev->mode_config.degamma_lut_property = prop; > + > + prop = drm_property_create_range(dev, > + DRM_MODE_PROP_IMMUTABLE, > + "DEGAMMA_LUT_SIZE", 0, UINT_MAX); > + if (!prop) > + return -ENOMEM; > + dev->mode_config.degamma_lut_size_property = prop; > + > + prop = drm_property_create(dev, > + DRM_MODE_PROP_BLOB, > + "CTM_MATRIX", 0); > + if (!prop) > + return -ENOMEM; > + dev->mode_config.ctm_matrix_property = prop; > + > + prop = drm_property_create(dev, > + DRM_MODE_PROP_BLOB, > + "GAMMA_LUT", 0); > + if (!prop) > + return -ENOMEM; > + dev->mode_config.gamma_lut_property = prop; > + > + prop = drm_property_create_range(dev, > + DRM_MODE_PROP_IMMUTABLE, > + "GAMMA_LUT_SIZE", 0, UINT_MAX); > + if (!prop) > + return -ENOMEM; > + dev->mode_config.gamma_lut_size_property = prop; > + > return 0; > } > > diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c > index 5d4bc64..b9b1929 100644 > --- a/drivers/gpu/drm/drm_crtc_helper.c > +++ b/drivers/gpu/drm/drm_crtc_helper.c > @@ -1064,3 +1064,36 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, > return drm_plane_helper_commit(plane, plane_state, old_fb); > } > EXPORT_SYMBOL(drm_helper_crtc_mode_set_base); > + > +/** > + * drm_helper_crtc_enable_color_mgmt - enable color management properties > + * @crtc: DRM CRTC > + * @degamma_lut_size: the size of the degamma lut (before CSC) > + * @gamma_lut_size: the size of the gamma lut (after CSC) > + * > + * This function lets the driver enable the color correction properties on a > + * CRTC. This includes 3 degamma, csc and gamma properties that userspace can > + * set and 2 size properties to inform the userspace of the lut sizes. > + */ > +void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc, > + int degamma_lut_size, > + int gamma_lut_size) > +{ > + struct drm_device *dev = crtc->dev; > + struct drm_mode_config *config = &dev->mode_config; > + > + drm_object_attach_property(&crtc->base, > + config->degamma_lut_property, 0); > + drm_object_attach_property(&crtc->base, > + config->ctm_matrix_property, 0); > + drm_object_attach_property(&crtc->base, > + config->gamma_lut_property, 0); > + > + drm_object_attach_property(&crtc->base, > + config->degamma_lut_size_property, > + degamma_lut_size); > + drm_object_attach_property(&crtc->base, > + config->gamma_lut_size_property, > + gamma_lut_size); > +} > +EXPORT_SYMBOL(drm_helper_crtc_enable_color_mgmt); > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index c65a212..967d646 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -305,12 +305,19 @@ struct drm_plane_helper_funcs; > * @mode_changed: crtc_state->mode or crtc_state->enable has been changed > * @active_changed: crtc_state->active has been toggled. > * @connectors_changed: connectors to this crtc have been updated > + * @color_mgmt_changed: color management properties have changed (degamma or > + * gamma LUT or CSC matrix) > * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes > * @connector_mask: bitmask of (1 << drm_connector_index(connector)) of attached connectors > * @last_vblank_count: for helpers and drivers to capture the vblank of the > * update to ensure framebuffer cleanup isn't done too early > * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings > * @mode: current mode timings > + * @degamma_lut: Lookup table for converting framebuffer pixel data > + * before apply the conversion matrix > + * @ctm_matrix: Transformation matrix > + * @gamma_lut: Lookup table for converting pixel data after the > + * conversion matrix > * @event: optional pointer to a DRM event to signal upon completion of the > * state update > * @state: backpointer to global drm_atomic_state > @@ -332,6 +339,7 @@ struct drm_crtc_state { > bool mode_changed : 1; > bool active_changed : 1; > bool connectors_changed : 1; > + bool color_mgmt_changed : 1; > > /* attached planes bitmask: > * WARNING: transitional helpers do not maintain plane_mask so > @@ -353,6 +361,11 @@ struct drm_crtc_state { > /* blob property to expose current mode to atomic userspace */ > struct drm_property_blob *mode_blob; > > + /* blob property to expose color management to userspace */ > + struct drm_property_blob *degamma_lut; > + struct drm_property_blob *ctm_matrix; > + struct drm_property_blob *gamma_lut; > + > struct drm_pending_vblank_event *event; > > struct drm_atomic_state *state; > @@ -755,7 +768,7 @@ struct drm_crtc { > int x, y; > const struct drm_crtc_funcs *funcs; > > - /* CRTC gamma size for reporting to userspace */ > + /* Legacy FB CRTC gamma size for reporting to userspace */ > uint32_t gamma_size; > uint16_t *gamma_store; > > @@ -2023,6 +2036,15 @@ struct drm_mode_config_funcs { > * @property_blob_list: list of all the blob property objects > * @blob_lock: mutex for blob property allocation and management > * @*_property: core property tracking > + * @degamma_lut_property: LUT used to convert the framebuffer's colors to linear > + * gamma > + * @degamma_lut_size_property: size of the degamma LUT as supported by the > + * driver (read-only) > + * @ctm_matrix_property: Matrix used to convert colors after the lookup in the > + * degamma LUT > + * @gamma_lut_property: LUT used to convert the colors, after the CSC matrix, to > + * the gamma space of the connected screen (read-only) > + * @gamma_lut_size_property: size of the gamma LUT as supported by the driver > * @preferred_depth: preferred RBG pixel depth, used by fb helpers > * @prefer_shadow: hint to userspace to prefer shadow-fb rendering > * @async_page_flip: does this device support async flips on the primary plane? > @@ -2124,6 +2146,13 @@ struct drm_mode_config { > struct drm_property *aspect_ratio_property; > struct drm_property *dirty_info_property; > > + /* Optional color correction properties */ > + struct drm_property *degamma_lut_property; > + struct drm_property *degamma_lut_size_property; > + struct drm_property *ctm_matrix_property; > + struct drm_property *gamma_lut_property; > + struct drm_property *gamma_lut_size_property; > + > /* properties for virtual machine layout */ > struct drm_property *suggested_x_property; > struct drm_property *suggested_y_property; > @@ -2530,6 +2559,18 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev, > return mo ? obj_to_property(mo) : NULL; > } > > +/* > + * Extract a degamma/gamma LUT value provided by user and round it to the > + * precision supported by the hardware. > + */ > +static inline uint32_t drm_color_lut_get_value(uint32_t user_input, > + uint32_t bit_precision) > +{ > + uint32_t val = (user_input << 1) + (1 << (16 - bit_precision)); > + > + return val >> (16 - bit_precision + 1); > +} > + > /* Plane list iterator for legacy (overlay only) planes. */ > #define drm_for_each_legacy_plane(plane, dev) \ > list_for_each_entry(plane, &(dev)->mode_config.plane_list, head) \ > diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h > index 4b37afa..97fa894 100644 > --- a/include/drm/drm_crtc_helper.h > +++ b/include/drm/drm_crtc_helper.h > @@ -48,6 +48,9 @@ extern bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, > struct drm_display_mode *mode, > int x, int y, > struct drm_framebuffer *old_fb); > +extern void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc, > + int degamma_lut_size, > + int gamma_lut_size); > extern bool drm_helper_crtc_in_use(struct drm_crtc *crtc); > extern bool drm_helper_encoder_in_use(struct drm_encoder *encoder); > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > index 50adb46..c021743 100644 > --- a/include/uapi/drm/drm_mode.h > +++ b/include/uapi/drm/drm_mode.h > @@ -487,6 +487,21 @@ struct drm_mode_crtc_lut { > __u64 blue; > }; > > +struct drm_color_ctm { > + /* Conversion matrix in S31.32 format. */ > + __s64 matrix[9]; > +}; > + > +struct drm_color_lut { > + /* > + * Data is U0.16 fixed point format. > + */ > + __u16 red; > + __u16 green; > + __u16 blue; > + __u16 reserved; > +}; > + > #define DRM_MODE_PAGE_FLIP_EVENT 0x01 > #define DRM_MODE_PAGE_FLIP_ASYNC 0x02 > #define DRM_MODE_PAGE_FLIP_FLAGS (DRM_MODE_PAGE_FLIP_EVENT|DRM_MODE_PAGE_FLIP_ASYNC) > -- > 2.6.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi, On 22 January 2016 at 21:20, Matt Roper <matthew.d.roper@intel.com> wrote: > Probably should have noticed/commented on this on your previous > iteration, but should we also restrict these new properties to be > atomic-only? I thought there was a consensus a while back that new > functionality would only be exposed for atomic-ready userspace. That > would mean also adding DRM_MODE_PROP_ATOMIC to your flags at creation. > Sorry for not noticing this when I commented before! I don't really see a reason to mandate atomic for these. They'll still work with normal prop sets, every bit as well/badly as pre-atomic has always worked. Cheers, Daniel
On Mon, Jan 25, 2016 at 10:57:51AM +0000, Daniel Stone wrote: > Hi, > > On 22 January 2016 at 21:20, Matt Roper <matthew.d.roper@intel.com> wrote: > > Probably should have noticed/commented on this on your previous > > iteration, but should we also restrict these new properties to be > > atomic-only? I thought there was a consensus a while back that new > > functionality would only be exposed for atomic-ready userspace. That > > would mean also adding DRM_MODE_PROP_ATOMIC to your flags at creation. > > Sorry for not noticing this when I commented before! > > I don't really see a reason to mandate atomic for these. They'll still > work with normal prop sets, every bit as well/badly as pre-atomic has > always worked. > Yeah, I agree that these could still work for legacy. But I thought the goal was to make new common properties atomic-only to help push atomic as the API of the future and also to reduce the possibility of new kernel properties confusing any poorly-written existing userspace. I thought danvet had written something about that a while back, but I could be completely misremembering. If the consensus is that exposing these for non-atomic is okay, then I don't have any serious objections to that. Thanks. Matt > Cheers, > Daniel
diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl index 351e801..c17ac29 100644 --- a/Documentation/DocBook/gpu.tmpl +++ b/Documentation/DocBook/gpu.tmpl @@ -2068,7 +2068,7 @@ void intel_crt_init(struct drm_device *dev) <td valign="top" >property to suggest an Y offset for a connector</td> </tr> <tr> - <td rowspan="3" valign="top" >Optional</td> + <td rowspan="8" valign="top" >Optional</td> <td valign="top" >“scaling mode”</td> <td valign="top" >ENUM</td> <td valign="top" >{ "None", "Full", "Center", "Full aspect" }</td> @@ -2092,6 +2092,54 @@ void intel_crt_init(struct drm_device *dev) <td valign="top" >TBD</td> </tr> <tr> + <td valign="top" >“DEGAMMA_LUT”</td> + <td valign="top" >BLOB</td> + <td valign="top" >0</td> + <td valign="top" >CRTC</td> + <td valign="top" >DRM property to set the degamma LUT mapping + pixel data from the framebuffer before it is given to the + transformation matrix. The data is an interpreted as an array + of struct drm_color_lut elements.</td> + </tr> + <tr> + <td valign="top" >“DEGAMMA_LUT_SIZE”</td> + <td valign="top" >RANGE | IMMUTABLE</td> + <td valign="top" >Min=0, Max=UINT_MAX</td> + <td valign="top" >CRTC</td> + <td valign="top" >DRM property to gives the size of the LUT to + be set on the DEGAMMA_LUT property (the size depends on the + underlying hardware).</td> + </tr> + <tr> + <td valign="top" >“CTM_MATRIX”</td> + <td valign="top" >BLOB</td> + <td valign="top" >0</td> + <td valign="top" >CRTC</td> + <td valign="top" >DRM property to set the transformation + matrix apply to pixel data after the lookup through the + degamma LUT and before the lookup through the gamma LUT. The + data is an interpreted as a struct drm_color_ctm.</td> + </tr> + <tr> + <td valign="top" >“GAMMA_LUT”</td> + <td valign="top" >BLOB</td> + <td valign="top" >0</td> + <td valign="top" >CRTC</td> + <td valign="top" >DRM property to set the gamma LUT mapping + pixel data after to the transformation matrix to data sent to + the connector. The data is an interpreted as an array + of struct drm_color_lut elements.</td> + </tr> + <tr> + <td valign="top" >“GAMMA_LUT_SIZE”</td> + <td valign="top" >RANGE | IMMUTABLE</td> + <td valign="top" >Min=0, Max=UINT_MAX</td> + <td valign="top" >CRTC</td> + <td valign="top" >DRM property to gives the size of the LUT to + be set on the GAMMA_LUT property (the size depends on the + underlying hardware).</td> + </tr> + <tr> <td rowspan="20" valign="top" >i915</td> <td rowspan="2" valign="top" >Generic</td> <td valign="top" >"Broadcast RGB"</td> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 3f74193..5287416 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -28,6 +28,7 @@ #include <drm/drmP.h> #include <drm/drm_atomic.h> +#include <drm/drm_mode.h> #include <drm/drm_plane_helper.h> /** @@ -388,6 +389,58 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state, EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc); /** + * drm_atomic_replace_property_blob - replace a blob property + * @blob: a pointer to the member blob to be replaced + * @new_blob: the new blob to replace with + * @expected_size: the expected size of the new blob + * @replaced: whether the blob has been replaced + * + * RETURNS: + * Zero on success, error code on failure + */ +static int +drm_atomic_replace_property_blob(struct drm_property_blob **blob, + struct drm_property_blob *new_blob, + size_t expected_size, + bool *replaced) +{ + struct drm_property_blob *old_blob = *blob; + + if (old_blob == new_blob) + return 0; + + if (new_blob != NULL && new_blob->length != expected_size) + return -EINVAL; + + if (old_blob != NULL) + drm_property_unreference_blob(old_blob); + *blob = new_blob; + *replaced = true; + + return 0; +} + +static int +drm_atomic_replace_property_blob_from_id(struct drm_crtc *crtc, + struct drm_property_blob **blob, + uint64_t blob_id, + size_t expected_size, + bool *replaced) +{ + struct drm_device *dev = crtc->dev; + struct drm_property_blob *new_blob = NULL; + + if (blob_id != 0) { + new_blob = drm_property_lookup_blob(dev, blob_id); + if (new_blob == NULL) + return -EINVAL; + } + + return drm_atomic_replace_property_blob(blob, new_blob, + expected_size, replaced); +} + +/** * drm_atomic_crtc_set_property - set property on CRTC * @crtc: the drm CRTC to set a property on * @state: the state object to update with the new property value @@ -419,8 +472,47 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc, ret = drm_atomic_set_mode_prop_for_crtc(state, mode); drm_property_unreference_blob(mode); return ret; - } - else if (crtc->funcs->atomic_set_property) + } else if (property == config->degamma_lut_property) { + bool replaced = false; + uint64_t lut_size = 0; + + ret = drm_object_property_get_value(&crtc->base, + config->degamma_lut_size_property, + &lut_size); + if (ret == 0) + ret = drm_atomic_replace_property_blob_from_id(crtc, + &state->degamma_lut, + val, + lut_size * sizeof(struct drm_color_lut), + &replaced); + state->color_mgmt_changed = replaced; + return ret; + } else if (property == config->ctm_matrix_property) { + bool replaced = false; + + ret = drm_atomic_replace_property_blob_from_id(crtc, + &state->ctm_matrix, + val, + sizeof(struct drm_color_ctm), + &replaced); + state->color_mgmt_changed = replaced; + return ret; + } else if (property == config->gamma_lut_property) { + bool replaced = false; + uint64_t lut_size = 0; + + ret = drm_object_property_get_value(&crtc->base, + config->gamma_lut_size_property, + &lut_size); + if (ret == 0) + ret = drm_atomic_replace_property_blob_from_id(crtc, + &state->gamma_lut, + val, + lut_size * sizeof(struct drm_color_lut), + &replaced); + state->color_mgmt_changed = replaced; + return ret; + } else if (crtc->funcs->atomic_set_property) return crtc->funcs->atomic_set_property(crtc, state, property, val); else return -EINVAL; @@ -456,6 +548,12 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc, *val = state->active; else if (property == config->prop_mode_id) *val = (state->mode_blob) ? state->mode_blob->base.id : 0; + else if (property == config->degamma_lut_property) + *val = (state->degamma_lut) ? state->degamma_lut->base.id : 0; + else if (property == config->ctm_matrix_property) + *val = (state->ctm_matrix) ? state->ctm_matrix->base.id : 0; + else if (property == config->gamma_lut_property) + *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0; else if (crtc->funcs->atomic_get_property) return crtc->funcs->atomic_get_property(crtc, state, property, val); else diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index f0c3984..e4a5755 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2454,10 +2454,17 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc, if (state->mode_blob) drm_property_reference_blob(state->mode_blob); + if (state->degamma_lut) + drm_property_reference_blob(state->degamma_lut); + if (state->ctm_matrix) + drm_property_reference_blob(state->ctm_matrix); + if (state->gamma_lut) + drm_property_reference_blob(state->gamma_lut); state->mode_changed = false; state->active_changed = false; state->planes_changed = false; state->connectors_changed = false; + state->color_mgmt_changed = false; state->event = NULL; } EXPORT_SYMBOL(__drm_atomic_helper_crtc_duplicate_state); @@ -2498,6 +2505,9 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc, struct drm_crtc_state *state) { drm_property_unreference_blob(state->mode_blob); + drm_property_unreference_blob(state->degamma_lut); + drm_property_unreference_blob(state->ctm_matrix); + drm_property_unreference_blob(state->gamma_lut); } EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state); diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d40bab2..b1aa38a 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1542,6 +1542,41 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.prop_mode_id = prop; + prop = drm_property_create(dev, + DRM_MODE_PROP_BLOB, + "DEGAMMA_LUT", 0); + if (!prop) + return -ENOMEM; + dev->mode_config.degamma_lut_property = prop; + + prop = drm_property_create_range(dev, + DRM_MODE_PROP_IMMUTABLE, + "DEGAMMA_LUT_SIZE", 0, UINT_MAX); + if (!prop) + return -ENOMEM; + dev->mode_config.degamma_lut_size_property = prop; + + prop = drm_property_create(dev, + DRM_MODE_PROP_BLOB, + "CTM_MATRIX", 0); + if (!prop) + return -ENOMEM; + dev->mode_config.ctm_matrix_property = prop; + + prop = drm_property_create(dev, + DRM_MODE_PROP_BLOB, + "GAMMA_LUT", 0); + if (!prop) + return -ENOMEM; + dev->mode_config.gamma_lut_property = prop; + + prop = drm_property_create_range(dev, + DRM_MODE_PROP_IMMUTABLE, + "GAMMA_LUT_SIZE", 0, UINT_MAX); + if (!prop) + return -ENOMEM; + dev->mode_config.gamma_lut_size_property = prop; + return 0; } diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 5d4bc64..b9b1929 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -1064,3 +1064,36 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, return drm_plane_helper_commit(plane, plane_state, old_fb); } EXPORT_SYMBOL(drm_helper_crtc_mode_set_base); + +/** + * drm_helper_crtc_enable_color_mgmt - enable color management properties + * @crtc: DRM CRTC + * @degamma_lut_size: the size of the degamma lut (before CSC) + * @gamma_lut_size: the size of the gamma lut (after CSC) + * + * This function lets the driver enable the color correction properties on a + * CRTC. This includes 3 degamma, csc and gamma properties that userspace can + * set and 2 size properties to inform the userspace of the lut sizes. + */ +void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc, + int degamma_lut_size, + int gamma_lut_size) +{ + struct drm_device *dev = crtc->dev; + struct drm_mode_config *config = &dev->mode_config; + + drm_object_attach_property(&crtc->base, + config->degamma_lut_property, 0); + drm_object_attach_property(&crtc->base, + config->ctm_matrix_property, 0); + drm_object_attach_property(&crtc->base, + config->gamma_lut_property, 0); + + drm_object_attach_property(&crtc->base, + config->degamma_lut_size_property, + degamma_lut_size); + drm_object_attach_property(&crtc->base, + config->gamma_lut_size_property, + gamma_lut_size); +} +EXPORT_SYMBOL(drm_helper_crtc_enable_color_mgmt); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index c65a212..967d646 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -305,12 +305,19 @@ struct drm_plane_helper_funcs; * @mode_changed: crtc_state->mode or crtc_state->enable has been changed * @active_changed: crtc_state->active has been toggled. * @connectors_changed: connectors to this crtc have been updated + * @color_mgmt_changed: color management properties have changed (degamma or + * gamma LUT or CSC matrix) * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes * @connector_mask: bitmask of (1 << drm_connector_index(connector)) of attached connectors * @last_vblank_count: for helpers and drivers to capture the vblank of the * update to ensure framebuffer cleanup isn't done too early * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings * @mode: current mode timings + * @degamma_lut: Lookup table for converting framebuffer pixel data + * before apply the conversion matrix + * @ctm_matrix: Transformation matrix + * @gamma_lut: Lookup table for converting pixel data after the + * conversion matrix * @event: optional pointer to a DRM event to signal upon completion of the * state update * @state: backpointer to global drm_atomic_state @@ -332,6 +339,7 @@ struct drm_crtc_state { bool mode_changed : 1; bool active_changed : 1; bool connectors_changed : 1; + bool color_mgmt_changed : 1; /* attached planes bitmask: * WARNING: transitional helpers do not maintain plane_mask so @@ -353,6 +361,11 @@ struct drm_crtc_state { /* blob property to expose current mode to atomic userspace */ struct drm_property_blob *mode_blob; + /* blob property to expose color management to userspace */ + struct drm_property_blob *degamma_lut; + struct drm_property_blob *ctm_matrix; + struct drm_property_blob *gamma_lut; + struct drm_pending_vblank_event *event; struct drm_atomic_state *state; @@ -755,7 +768,7 @@ struct drm_crtc { int x, y; const struct drm_crtc_funcs *funcs; - /* CRTC gamma size for reporting to userspace */ + /* Legacy FB CRTC gamma size for reporting to userspace */ uint32_t gamma_size; uint16_t *gamma_store; @@ -2023,6 +2036,15 @@ struct drm_mode_config_funcs { * @property_blob_list: list of all the blob property objects * @blob_lock: mutex for blob property allocation and management * @*_property: core property tracking + * @degamma_lut_property: LUT used to convert the framebuffer's colors to linear + * gamma + * @degamma_lut_size_property: size of the degamma LUT as supported by the + * driver (read-only) + * @ctm_matrix_property: Matrix used to convert colors after the lookup in the + * degamma LUT + * @gamma_lut_property: LUT used to convert the colors, after the CSC matrix, to + * the gamma space of the connected screen (read-only) + * @gamma_lut_size_property: size of the gamma LUT as supported by the driver * @preferred_depth: preferred RBG pixel depth, used by fb helpers * @prefer_shadow: hint to userspace to prefer shadow-fb rendering * @async_page_flip: does this device support async flips on the primary plane? @@ -2124,6 +2146,13 @@ struct drm_mode_config { struct drm_property *aspect_ratio_property; struct drm_property *dirty_info_property; + /* Optional color correction properties */ + struct drm_property *degamma_lut_property; + struct drm_property *degamma_lut_size_property; + struct drm_property *ctm_matrix_property; + struct drm_property *gamma_lut_property; + struct drm_property *gamma_lut_size_property; + /* properties for virtual machine layout */ struct drm_property *suggested_x_property; struct drm_property *suggested_y_property; @@ -2530,6 +2559,18 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev, return mo ? obj_to_property(mo) : NULL; } +/* + * Extract a degamma/gamma LUT value provided by user and round it to the + * precision supported by the hardware. + */ +static inline uint32_t drm_color_lut_get_value(uint32_t user_input, + uint32_t bit_precision) +{ + uint32_t val = (user_input << 1) + (1 << (16 - bit_precision)); + + return val >> (16 - bit_precision + 1); +} + /* Plane list iterator for legacy (overlay only) planes. */ #define drm_for_each_legacy_plane(plane, dev) \ list_for_each_entry(plane, &(dev)->mode_config.plane_list, head) \ diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h index 4b37afa..97fa894 100644 --- a/include/drm/drm_crtc_helper.h +++ b/include/drm/drm_crtc_helper.h @@ -48,6 +48,9 @@ extern bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode, int x, int y, struct drm_framebuffer *old_fb); +extern void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc, + int degamma_lut_size, + int gamma_lut_size); extern bool drm_helper_crtc_in_use(struct drm_crtc *crtc); extern bool drm_helper_encoder_in_use(struct drm_encoder *encoder); diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 50adb46..c021743 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -487,6 +487,21 @@ struct drm_mode_crtc_lut { __u64 blue; }; +struct drm_color_ctm { + /* Conversion matrix in S31.32 format. */ + __s64 matrix[9]; +}; + +struct drm_color_lut { + /* + * Data is U0.16 fixed point format. + */ + __u16 red; + __u16 green; + __u16 blue; + __u16 reserved; +}; + #define DRM_MODE_PAGE_FLIP_EVENT 0x01 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02 #define DRM_MODE_PAGE_FLIP_FLAGS (DRM_MODE_PAGE_FLIP_EVENT|DRM_MODE_PAGE_FLIP_ASYNC)
v2: Register LUT size properties as range Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- Documentation/DocBook/gpu.tmpl | 50 +++++++++++++++++- drivers/gpu/drm/drm_atomic.c | 102 +++++++++++++++++++++++++++++++++++- drivers/gpu/drm/drm_atomic_helper.c | 10 ++++ drivers/gpu/drm/drm_crtc.c | 35 +++++++++++++ drivers/gpu/drm/drm_crtc_helper.c | 33 ++++++++++++ include/drm/drm_crtc.h | 43 ++++++++++++++- include/drm/drm_crtc_helper.h | 3 ++ include/uapi/drm/drm_mode.h | 15 ++++++ 8 files changed, 287 insertions(+), 4 deletions(-)