On Tue, 20 Aug 2019, Ramalingam C <ramalingam.c@xxxxxxxxx> wrote:
> On 2019-08-20 at 14:14:03 +0530, Winkler, Tomas wrote:
>>
>>
>> >
>> > For the reusability of the enum transcoder and enum pipe in other driver
>> > modules (like mei_hdcp), enum port definition is moved from I915 local header
>> > intel_display.h to drm/i915_drm.h
>>
>> Don't you need to name space those definitions in the global space, I guess there are a lot of 'pipe' variables and definitions you can conflict with.
>> I guess it should be enum i915_pipe, etc.
> I am assuming that this header will be used only when you we build for
> I915 driver(intel). In such case, we wont have a conflict.
>
> Unless until this renaming is needed, we shouldn't get into it, as this
> will demand i915 wide renaming.
I think we need to keep enum transcoder and enum pipe internal to i915,
as well as pull enum port from i915_drm.h back to i915 internal as
well. I don't think they should be exposed outside of i915.
I think it would be better to expose the existing enum mei_fw_ddi and
the new enum mei_fw_tc from MEI to i915 instead, and have i915 fill in
the relevant physical_port (ddi index) and attached_transcoder (tc
type). I don't know what these members should be called in struct
hdcp_port_data, but the point is that i915 would do the translation, not
mei_hdcp.c. The types of these members should obviously be enum
meo_fw_ddi and enum mei_fw_tc.
Thoughts?
BR,
Jani.
>
> -Ram
>>
>> Thanks
>> Tomas
>>
>>
>> >
>> > Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx>
>> > ---
>> > drivers/gpu/drm/i915/display/intel_display.h | 44 -------------------
>> > include/drm/i915_drm.h | 46 ++++++++++++++++++++
>> > 2 files changed, 46 insertions(+), 44 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_display.h
>> > b/drivers/gpu/drm/i915/display/intel_display.h
>> > index e57e6969051d..56f3d9073159 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_display.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
>> > @@ -71,50 +71,6 @@ enum i915_gpio {
>> > GPIOO,
>> > };
>> >
>> > -/*
>> > - * Keep the pipe enum values fixed: the code assumes that PIPE_A=0, the
>> > - * rest have consecutive values and match the enum values of transcoders
>> > - * with a 1:1 transcoder -> pipe mapping.
>> > - */
>> > -enum pipe {
>> > - INVALID_PIPE = -1,
>> > -
>> > - PIPE_A = 0,
>> > - PIPE_B,
>> > - PIPE_C,
>> > - PIPE_D,
>> > - _PIPE_EDP,
>> > -
>> > - I915_MAX_PIPES = _PIPE_EDP
>> > -};
>> > -
>> > -#define pipe_name(p) ((p) + 'A')
>> > -
>> > -enum transcoder {
>> > - /*
>> > - * The following transcoders have a 1:1 transcoder -> pipe mapping,
>> > - * keep their values fixed: the code assumes that TRANSCODER_A=0,
>> > the
>> > - * rest have consecutive values and match the enum values of the pipes
>> > - * they map to.
>> > - */
>> > - TRANSCODER_A = PIPE_A,
>> > - TRANSCODER_B = PIPE_B,
>> > - TRANSCODER_C = PIPE_C,
>> > - TRANSCODER_D = PIPE_D,
>> > -
>> > - /*
>> > - * The following transcoders can map to any pipe, their enum value
>> > - * doesn't need to stay fixed.
>> > - */
>> > - TRANSCODER_EDP,
>> > - TRANSCODER_DSI_0,
>> > - TRANSCODER_DSI_1,
>> > - TRANSCODER_DSI_A = TRANSCODER_DSI_0, /* legacy DSI */
>> > - TRANSCODER_DSI_C = TRANSCODER_DSI_1, /* legacy DSI */
>> > -
>> > - I915_MAX_TRANSCODERS
>> > -};
>> > -
>> > static inline const char *transcoder_name(enum transcoder transcoder) {
>> > switch (transcoder) {
>> > diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h index
>> > 23274cf92712..b0779b8267b9 100644
>> > --- a/include/drm/i915_drm.h
>> > +++ b/include/drm/i915_drm.h
>> > @@ -118,4 +118,50 @@ enum port {
>> >
>> > #define port_name(p) ((p) + 'A')
>> >
>> > +/*
>> > + * Keep the pipe enum values fixed: the code assumes that PIPE_A=0, the
>> > + * rest have consecutive values and match the enum values of
>> > +transcoders
>> > + * with a 1:1 transcoder -> pipe mapping.
>> > + */
>> > +enum pipe {
>> > + INVALID_PIPE = -1,
>> > +
>> > + PIPE_A = 0,
>> > + PIPE_B,
>> > + PIPE_C,
>> > + PIPE_D,
>> > + _PIPE_EDP,
>> > +
>> > + I915_MAX_PIPES = _PIPE_EDP
>> > +};
>> > +
>> > +#define pipe_name(p) ((p) + 'A')
>> > +
>> > +enum transcoder {
>> > + INVALID_TRANSCODER = -1,
>> > +
>> > + /*
>> > + * The following transcoders have a 1:1 transcoder -> pipe mapping,
>> > + * keep their values fixed: the code assumes that TRANSCODER_A=0,
>> > the
>> > + * rest have consecutive values and match the enum values of the pipes
>> > + * they map to.
>> > + */
>> > + TRANSCODER_A = PIPE_A,
>> > + TRANSCODER_B = PIPE_B,
>> > + TRANSCODER_C = PIPE_C,
>> > + TRANSCODER_D = PIPE_D,
>> > +
>> > + /*
>> > + * The following transcoders can map to any pipe, their enum value
>> > + * doesn't need to stay fixed.
>> > + */
>> > + TRANSCODER_EDP,
>> > + TRANSCODER_DSI_0,
>> > + TRANSCODER_DSI_1,
>> > + TRANSCODER_DSI_A = TRANSCODER_DSI_0, /* legacy DSI */
>> > + TRANSCODER_DSI_C = TRANSCODER_DSI_1, /* legacy DSI */
>> > +
>> > + I915_MAX_TRANSCODERS
>> > +};
>> > +
>> > #endif /* _I915_DRM_H_ */
>> > --
>> > 2.20.1
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
