| Message ID | 20180626062259.15563-9-mahesh1.kumar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 26, 2018 at 11:52:57AM +0530, Mahesh Kumar wrote: > This patch implements get_crc_sources callback, which returns list of > all the valid crc sources supported by driver in current platform. > > Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 1 + > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_pipe_crc.c | 12 ++++++++++++ > 3 files changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index c0eb752b0901..93b4be2eee32 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -12894,6 +12894,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = { > .atomic_destroy_state = intel_crtc_destroy_state, > .set_crc_source = intel_crtc_set_crc_source, > .verify_crc_source = intel_crtc_verify_crc_source, > + .get_crc_sources = intel_crtc_get_crc_sources, > }; > > struct wait_rps_boost { > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 64e13adad9f5..17735cafdd72 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -2156,11 +2156,13 @@ int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name, > size_t *values_cnt); > int intel_crtc_verify_crc_source(struct drm_crtc *crtc, > const char *source_name, size_t *values_cnt); > +void intel_crtc_get_crc_sources(struct seq_file *m, struct drm_crtc *crtc); > void intel_crtc_disable_pipe_crc(struct intel_crtc *crtc); > void intel_crtc_enable_pipe_crc(struct intel_crtc *crtc); > #else > #define intel_crtc_set_crc_source NULL > #define intel_crtc_verify_crc_source NULL > +#define intel_crtc_get_crc_sources NULL > static inline void intel_crtc_disable_pipe_crc(struct intel_crtc *crtc) > { > } > diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c > index a37521380f94..d6807155a237 100644 > --- a/drivers/gpu/drm/i915/intel_pipe_crc.c > +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c > @@ -1001,6 +1001,18 @@ intel_is_valid_crc_source(struct drm_i915_private *dev_priv, > return ivb_crc_source_valid(dev_priv, source); > } > > +void intel_crtc_get_crc_sources(struct seq_file *m, struct drm_crtc *crtc) > +{ > + struct drm_i915_private *dev_priv = to_i915(crtc->dev); > + enum intel_pipe_crc_source source; > + > + seq_puts(m, "["); > + for (source = 0; source < INTEL_PIPE_CRC_SOURCE_MAX; source++) > + if (intel_is_valid_crc_source(dev_priv, source) == 0) > + seq_printf(m, "%s,", pipe_crc_source_name(source)); > + seq_puts(m, "auto] "); This seems to be a very quirky interface ... Can't you instead return a real array, and then also push validation into the core? Same we do with e.g. the per-plane drm_fourcc format support. Something like an array of const char * pointers should be good. -Daniel > +} > + > int intel_crtc_verify_crc_source(struct drm_crtc *crtc, const char *source_name, > size_t *values_cnt) > { > -- > 2.16.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi, thanks for review. On 6/26/2018 1:59 PM, Daniel Vetter wrote: > On Tue, Jun 26, 2018 at 11:52:57AM +0530, Mahesh Kumar wrote: >> This patch implements get_crc_sources callback, which returns list of >> all the valid crc sources supported by driver in current platform. >> >> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com> >> --- >> drivers/gpu/drm/i915/intel_display.c | 1 + >> drivers/gpu/drm/i915/intel_drv.h | 2 ++ >> drivers/gpu/drm/i915/intel_pipe_crc.c | 12 ++++++++++++ >> 3 files changed, 15 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index c0eb752b0901..93b4be2eee32 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -12894,6 +12894,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = { >> .atomic_destroy_state = intel_crtc_destroy_state, >> .set_crc_source = intel_crtc_set_crc_source, >> .verify_crc_source = intel_crtc_verify_crc_source, >> + .get_crc_sources = intel_crtc_get_crc_sources, >> }; >> >> struct wait_rps_boost { >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> index 64e13adad9f5..17735cafdd72 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -2156,11 +2156,13 @@ int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name, >> size_t *values_cnt); >> int intel_crtc_verify_crc_source(struct drm_crtc *crtc, >> const char *source_name, size_t *values_cnt); >> +void intel_crtc_get_crc_sources(struct seq_file *m, struct drm_crtc *crtc); >> void intel_crtc_disable_pipe_crc(struct intel_crtc *crtc); >> void intel_crtc_enable_pipe_crc(struct intel_crtc *crtc); >> #else >> #define intel_crtc_set_crc_source NULL >> #define intel_crtc_verify_crc_source NULL >> +#define intel_crtc_get_crc_sources NULL >> static inline void intel_crtc_disable_pipe_crc(struct intel_crtc *crtc) >> { >> } >> diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c >> index a37521380f94..d6807155a237 100644 >> --- a/drivers/gpu/drm/i915/intel_pipe_crc.c >> +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c >> @@ -1001,6 +1001,18 @@ intel_is_valid_crc_source(struct drm_i915_private *dev_priv, >> return ivb_crc_source_valid(dev_priv, source); >> } >> >> +void intel_crtc_get_crc_sources(struct seq_file *m, struct drm_crtc *crtc) >> +{ >> + struct drm_i915_private *dev_priv = to_i915(crtc->dev); >> + enum intel_pipe_crc_source source; >> + >> + seq_puts(m, "["); >> + for (source = 0; source < INTEL_PIPE_CRC_SOURCE_MAX; source++) >> + if (intel_is_valid_crc_source(dev_priv, source) == 0) >> + seq_printf(m, "%s,", pipe_crc_source_name(source)); >> + seq_puts(m, "auto] "); > This seems to be a very quirky interface ... Can't you instead return a > real array, and then also push validation into the core? Same we do with > e.g. the per-plane drm_fourcc format support. Something like an array of > const char * pointers should be good. sounds good, will make the changes and return constant pointer to an array. Will also push validation to the crc-core -Mahesh > -Daniel > >> +} >> + >> int intel_crtc_verify_crc_source(struct drm_crtc *crtc, const char *source_name, >> size_t *values_cnt) >> { >> -- >> 2.16.2 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Jun 27, 2018 at 07:41:05PM +0530, Kumar, Mahesh wrote: > Hi, > > thanks for review. > On 6/26/2018 1:59 PM, Daniel Vetter wrote: > > On Tue, Jun 26, 2018 at 11:52:57AM +0530, Mahesh Kumar wrote: > > > This patch implements get_crc_sources callback, which returns list of > > > all the valid crc sources supported by driver in current platform. > > > > > > Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 1 + > > > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > > > drivers/gpu/drm/i915/intel_pipe_crc.c | 12 ++++++++++++ > > > 3 files changed, 15 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index c0eb752b0901..93b4be2eee32 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -12894,6 +12894,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = { > > > .atomic_destroy_state = intel_crtc_destroy_state, > > > .set_crc_source = intel_crtc_set_crc_source, > > > .verify_crc_source = intel_crtc_verify_crc_source, > > > + .get_crc_sources = intel_crtc_get_crc_sources, > > > }; > > > struct wait_rps_boost { > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > > index 64e13adad9f5..17735cafdd72 100644 > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > @@ -2156,11 +2156,13 @@ int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name, > > > size_t *values_cnt); > > > int intel_crtc_verify_crc_source(struct drm_crtc *crtc, > > > const char *source_name, size_t *values_cnt); > > > +void intel_crtc_get_crc_sources(struct seq_file *m, struct drm_crtc *crtc); > > > void intel_crtc_disable_pipe_crc(struct intel_crtc *crtc); > > > void intel_crtc_enable_pipe_crc(struct intel_crtc *crtc); > > > #else > > > #define intel_crtc_set_crc_source NULL > > > #define intel_crtc_verify_crc_source NULL > > > +#define intel_crtc_get_crc_sources NULL > > > static inline void intel_crtc_disable_pipe_crc(struct intel_crtc *crtc) > > > { > > > } > > > diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c > > > index a37521380f94..d6807155a237 100644 > > > --- a/drivers/gpu/drm/i915/intel_pipe_crc.c > > > +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c > > > @@ -1001,6 +1001,18 @@ intel_is_valid_crc_source(struct drm_i915_private *dev_priv, > > > return ivb_crc_source_valid(dev_priv, source); > > > } > > > +void intel_crtc_get_crc_sources(struct seq_file *m, struct drm_crtc *crtc) > > > +{ > > > + struct drm_i915_private *dev_priv = to_i915(crtc->dev); > > > + enum intel_pipe_crc_source source; > > > + > > > + seq_puts(m, "["); > > > + for (source = 0; source < INTEL_PIPE_CRC_SOURCE_MAX; source++) > > > + if (intel_is_valid_crc_source(dev_priv, source) == 0) > > > + seq_printf(m, "%s,", pipe_crc_source_name(source)); > > > + seq_puts(m, "auto] "); > > This seems to be a very quirky interface ... Can't you instead return a > > real array, and then also push validation into the core? Same we do with > > e.g. the per-plane drm_fourcc format support. Something like an array of > > const char * pointers should be good. > sounds good, will make the changes and return constant pointer to an array. > Will also push validation to the crc-core If you NULL terminate the array you can even skip the array_size. NULL terminating arrays is the usual linux pattern - we can't do it in a few places in drm (e.g. modifiers) because we don't have a canonical invalid value. Could/should probably fix that, but oh well. -Daniel > -Mahesh > > -Daniel > > > > > +} > > > + > > > int intel_crtc_verify_crc_source(struct drm_crtc *crtc, const char *source_name, > > > size_t *values_cnt) > > > { > > > -- > > > 2.16.2 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c0eb752b0901..93b4be2eee32 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12894,6 +12894,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = { .atomic_destroy_state = intel_crtc_destroy_state, .set_crc_source = intel_crtc_set_crc_source, .verify_crc_source = intel_crtc_verify_crc_source, + .get_crc_sources = intel_crtc_get_crc_sources, }; struct wait_rps_boost { diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 64e13adad9f5..17735cafdd72 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -2156,11 +2156,13 @@ int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name, size_t *values_cnt); int intel_crtc_verify_crc_source(struct drm_crtc *crtc, const char *source_name, size_t *values_cnt); +void intel_crtc_get_crc_sources(struct seq_file *m, struct drm_crtc *crtc); void intel_crtc_disable_pipe_crc(struct intel_crtc *crtc); void intel_crtc_enable_pipe_crc(struct intel_crtc *crtc); #else #define intel_crtc_set_crc_source NULL #define intel_crtc_verify_crc_source NULL +#define intel_crtc_get_crc_sources NULL static inline void intel_crtc_disable_pipe_crc(struct intel_crtc *crtc) { } diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c index a37521380f94..d6807155a237 100644 --- a/drivers/gpu/drm/i915/intel_pipe_crc.c +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c @@ -1001,6 +1001,18 @@ intel_is_valid_crc_source(struct drm_i915_private *dev_priv, return ivb_crc_source_valid(dev_priv, source); } +void intel_crtc_get_crc_sources(struct seq_file *m, struct drm_crtc *crtc) +{ + struct drm_i915_private *dev_priv = to_i915(crtc->dev); + enum intel_pipe_crc_source source; + + seq_puts(m, "["); + for (source = 0; source < INTEL_PIPE_CRC_SOURCE_MAX; source++) + if (intel_is_valid_crc_source(dev_priv, source) == 0) + seq_printf(m, "%s,", pipe_crc_source_name(source)); + seq_puts(m, "auto] "); +} + int intel_crtc_verify_crc_source(struct drm_crtc *crtc, const char *source_name, size_t *values_cnt) {
This patch implements get_crc_sources callback, which returns list of all the valid crc sources supported by driver in current platform. Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 1 + drivers/gpu/drm/i915/intel_drv.h | 2 ++ drivers/gpu/drm/i915/intel_pipe_crc.c | 12 ++++++++++++ 3 files changed, 15 insertions(+)