From: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH i-g-t 6/7] lib/igt_kms: rename igt_crtc_set_background
Date: Wed, 23 Mar 2016 14:39:26 +0000 [thread overview]
Message-ID: <56F2AA9E.3090501@intel.com> (raw)
In-Reply-To: <20160323122746.GQ28483@phenom.ffwll.local>
On 23/03/16 12:27, Daniel Vetter wrote:
> On Wed, Mar 23, 2016 at 11:38:09AM +0000, Lionel Landwerlin wrote:
>> We're a bit inconsistent with functions named igt_crtc_* and other
>> named igt_pipe_* even though they apply to the same objects.
>>
>> It seems most of the kernel/igt is using the pipe denomination. Let's
>> rename the igt_crtc_* into igt_pipe_*.
>>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Tbh I'd go the other direction. pipe is use in Bspec, and hence why it's
> used a lot in our i915 code. And also why we leaked it into igt_kms. But
> the drm thing a pipe is represented by is the CRTC. With others starting
> to use igt on non-intel drivers I think it'd be better to move into the
> other direction, and rename all the pipe stuff to CRTC. Or well most of it
> (I think we have some intel-specific concepts mixed in here).
> -Daniel
Thanks,
I also based my assumption on :
$ git grep igt_crtc_ | wc -l
19
$ git grep igt_pipe_ | wc -l
240
So maybe I'll submit another series for that as this would generate a
fair amount of noise :(
>> ---
>> lib/igt_kms.c | 28 ++++++++++++++--------------
>> lib/igt_kms.h | 29 ++++++++++++++---------------
>> tests/kms_crtc_background_color.c | 18 +++++++++---------
>> 3 files changed, 37 insertions(+), 38 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index e3488f1..1e0585f 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -160,7 +160,7 @@ static const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
>> "rotation"
>> };
>>
>> -static const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
>> +static const char *igt_pipe_prop_names[IGT_NUM_PIPE_PROPS] = {
>> "background_color",
>> "DEGAMMA_LUT",
>> "CTM",
>> @@ -1262,8 +1262,8 @@ void igt_display_init(igt_display_t *display, int drm_fd)
>> pipe->n_planes = p + 1;
>>
>> igt_atomic_fill_props(display, pipe->crtc_id, DRM_MODE_OBJECT_CRTC,
>> - pipe->properties, igt_crtc_prop_names,
>> - IGT_NUM_CRTC_PROPS);
>> + pipe->properties, igt_pipe_prop_names,
>> + IGT_NUM_PIPE_PROPS);
>> if (pipe->crtc_id) {
>> uint64_t prop_value;
>>
>> @@ -1835,7 +1835,7 @@ static int igt_output_commit(igt_output_t *output,
>>
>> if (pipe->background_changed) {
>> igt_crtc_set_property(output,
>> - pipe->properties[IGT_CRTC_BACKGROUND],
>> + pipe->properties[IGT_PIPE_BACKGROUND],
>> pipe->background);
>>
>> pipe->background_changed = false;
>> @@ -1843,11 +1843,11 @@ static int igt_output_commit(igt_output_t *output,
>>
>> if (pipe->color_mgmt_changed) {
>> igt_crtc_set_property(output,
>> - pipe->properties[IGT_CRTC_DEGAMMA_LUT],
>> + pipe->properties[IGT_PIPE_DEGAMMA_LUT],
>> pipe->degamma_blob);
>> - igt_crtc_set_property(output, pipe->properties[IGT_CRTC_CTM],
>> + igt_crtc_set_property(output, pipe->properties[IGT_PIPE_CTM],
>> pipe->ctm_blob);
>> - igt_crtc_set_property(output, pipe->properties[IGT_CRTC_GAMMA_LUT],
>> + igt_crtc_set_property(output, pipe->properties[IGT_PIPE_GAMMA_LUT],
>> pipe->gamma_blob);
>>
>> pipe->color_mgmt_changed = false;
>> @@ -1881,12 +1881,12 @@ static int igt_output_commit(igt_output_t *output,
>> static void igt_atomic_prepare_pipe_commit(igt_pipe_t *pipe, drmModeAtomicReq *req)
>> {
>> if (pipe->background_changed)
>> - igt_atomic_populate_pipe_req(req, pipe, IGT_CRTC_BACKGROUND, pipe->background);
>> + igt_atomic_populate_pipe_req(req, pipe, IGT_PIPE_BACKGROUND, pipe->background);
>>
>> if (pipe->color_mgmt_changed) {
>> - igt_atomic_populate_pipe_req(req, pipe, IGT_CRTC_DEGAMMA_LUT, pipe->degamma_blob);
>> - igt_atomic_populate_pipe_req(req, pipe, IGT_CRTC_CTM, pipe->ctm_blob);
>> - igt_atomic_populate_pipe_req(req, pipe, IGT_CRTC_GAMMA_LUT, pipe->gamma_blob);
>> + igt_atomic_populate_pipe_req(req, pipe, IGT_PIPE_DEGAMMA_LUT, pipe->degamma_blob);
>> + igt_atomic_populate_pipe_req(req, pipe, IGT_PIPE_CTM, pipe->ctm_blob);
>> + igt_atomic_populate_pipe_req(req, pipe, IGT_PIPE_GAMMA_LUT, pipe->gamma_blob);
>> }
>>
>> pipe->background_changed = false;
>> @@ -2316,7 +2316,7 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
>> }
>>
>> /**
>> - * igt_crtc_set_background:
>> + * igt_pipe_set_background:
>> * @pipe: pipe pointer to which background color to be set
>> * @background: background color value in BGR 16bpc
>> *
>> @@ -2325,11 +2325,11 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
>> * property.
>> * For example to get red as background, set background = 0x00000000FFFF.
>> */
>> -void igt_crtc_set_background(igt_pipe_t *pipe, uint64_t background)
>> +void igt_pipe_set_background(igt_pipe_t *pipe, uint64_t background)
>> {
>> igt_display_t *display = pipe->display;
>>
>> - LOG(display, "%s.%d: crtc_set_background(%"PRIx64")\n",
>> + LOG(display, "%s.%d: pipe_set_background(%"PRIx64")\n",
>> kmstest_pipe_name(pipe->pipe),
>> pipe->pipe, background);
>>
>> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>> index 4d3abf6..27c87ea 100644
>> --- a/lib/igt_kms.h
>> +++ b/lib/igt_kms.h
>> @@ -106,12 +106,12 @@ int kmstest_get_pipe_from_crtc_id(int fd, int crtc_id);
>> void kmstest_set_vt_graphics_mode(void);
>> void kmstest_restore_vt_mode(void);
>>
>> -enum igt_atomic_crtc_properties {
>> - IGT_CRTC_BACKGROUND = 0,
>> - IGT_CRTC_CTM,
>> - IGT_CRTC_DEGAMMA_LUT,
>> - IGT_CRTC_GAMMA_LUT,
>> - IGT_NUM_CRTC_PROPS
>> +enum igt_atomic_pipe_properties {
>> + IGT_PIPE_BACKGROUND = 0,
>> + IGT_PIPE_CTM,
>> + IGT_PIPE_DEGAMMA_LUT,
>> + IGT_PIPE_GAMMA_LUT,
>> + IGT_NUM_PIPE_PROPS
>> };
>>
>> enum igt_atomic_connector_properties {
>> @@ -262,7 +262,7 @@ struct igt_pipe {
>>
>> uint32_t crtc_id;
>>
>> - uint32_t properties[IGT_NUM_CRTC_PROPS];
>> + uint32_t properties[IGT_NUM_PIPE_PROPS];
>> };
>>
>> typedef struct {
>> @@ -305,6 +305,12 @@ igt_plane_t *igt_output_get_plane(igt_output_t *output, enum igt_plane plane);
>> bool igt_pipe_get_property(igt_pipe_t *pipe, const char *name,
>> uint32_t *prop_id, uint64_t *value,
>> drmModePropertyPtr *prop);
>> +void igt_pipe_set_background(igt_pipe_t *pipe, uint64_t background);
>> +
>> +static inline bool igt_pipe_supports_background(igt_pipe_t *pipe)
>> +{
>> + return pipe->properties[IGT_PIPE_BACKGROUND] != 0;
>> +}
>>
>> static inline bool igt_plane_supports_rotation(igt_plane_t *plane)
>> {
>> @@ -320,18 +326,11 @@ void igt_plane_set_position(igt_plane_t *plane, int x, int y);
>> void igt_plane_set_size(igt_plane_t *plane, int w, int h);
>> void igt_plane_set_panning(igt_plane_t *plane, int x, int y);
>> void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation);
>> -void igt_crtc_set_background(igt_pipe_t *pipe, uint64_t background);
>> void igt_fb_set_position(struct igt_fb *fb, igt_plane_t *plane,
>> uint32_t x, uint32_t y);
>> void igt_fb_set_size(struct igt_fb *fb, igt_plane_t *plane,
>> uint32_t w, uint32_t h);
>>
>> -static inline bool igt_pipe_supports_background(igt_pipe_t *pipe)
>> -{
>> - return pipe->properties[IGT_CRTC_BACKGROUND] != 0;
>> -}
>> -
>> -
>> void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>>
>> #define for_each_connected_output(display, output) \
>> @@ -362,7 +361,7 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>> * igt_atomic_populate_pipe_req:
>> * @req: A pointer to drmModeAtomicReq
>> * @pipe: A pointer igt_pipe_t
>> - * @prop: one of igt_atomic_crtc_properties
>> + * @prop: one of igt_atomic_pipe_properties
>> * @value: the value to add
>> */
>> #define igt_atomic_populate_pipe_req(req, pipe, prop, value) \
>> diff --git a/tests/kms_crtc_background_color.c b/tests/kms_crtc_background_color.c
>> index 055f935..0bf205e 100644
>> --- a/tests/kms_crtc_background_color.c
>> +++ b/tests/kms_crtc_background_color.c
>> @@ -97,7 +97,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
>> igt_assert(fb_id);
>>
>> /* To make FB pixel win with background color, set alpha as full opaque */
>> - igt_crtc_set_background(plane->pipe, pipe_background_color);
>> + igt_pipe_set_background(plane->pipe, pipe_background_color);
>> if (opaque_buffer)
>> alpha = 1.0; /* alpha 1 is fully opque */
>> else
>> @@ -117,7 +117,7 @@ static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
>>
>> igt_remove_fb(data->gfx_fd, &data->fb);
>>
>> - igt_crtc_set_background(plane->pipe, BLACK64);
>> + igt_pipe_set_background(plane->pipe, BLACK64);
>> igt_plane_set_fb(plane, NULL);
>> igt_output_set_pipe(output, PIPE_ANY);
>>
>> @@ -147,26 +147,26 @@ static void test_crtc_background(data_t *data)
>> /* Now set background without using a plane, i.e.,
>> * Disable the plane to let hw background color win blend. */
>> igt_plane_set_fb(plane, NULL);
>> - igt_crtc_set_background(plane->pipe, PURPLE64);
>> + igt_pipe_set_background(plane->pipe, PURPLE64);
>> igt_display_commit2(display, COMMIT_UNIVERSAL);
>>
>> /* Try few other background colors */
>> - igt_crtc_set_background(plane->pipe, CYAN64);
>> + igt_pipe_set_background(plane->pipe, CYAN64);
>> igt_display_commit2(display, COMMIT_UNIVERSAL);
>>
>> - igt_crtc_set_background(plane->pipe, YELLOW64);
>> + igt_pipe_set_background(plane->pipe, YELLOW64);
>> igt_display_commit2(display, COMMIT_UNIVERSAL);
>>
>> - igt_crtc_set_background(plane->pipe, RED64);
>> + igt_pipe_set_background(plane->pipe, RED64);
>> igt_display_commit2(display, COMMIT_UNIVERSAL);
>>
>> - igt_crtc_set_background(plane->pipe, GREEN64);
>> + igt_pipe_set_background(plane->pipe, GREEN64);
>> igt_display_commit2(display, COMMIT_UNIVERSAL);
>>
>> - igt_crtc_set_background(plane->pipe, BLUE64);
>> + igt_pipe_set_background(plane->pipe, BLUE64);
>> igt_display_commit2(display, COMMIT_UNIVERSAL);
>>
>> - igt_crtc_set_background(plane->pipe, WHITE64);
>> + igt_pipe_set_background(plane->pipe, WHITE64);
>> igt_display_commit2(display, COMMIT_UNIVERSAL);
>>
>> valid_tests++;
>> --
>> 2.8.0.rc3
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2016-03-23 14:39 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-23 11:38 [PATCH i-g-t 0/7] lib/igt_kms cleanups Lionel Landwerlin
2016-03-23 11:38 ` [PATCH i-g-t 1/7] lib/igt_kms: fix clearing up connector/pipe state on atomic commit Lionel Landwerlin
2016-03-23 11:38 ` [PATCH i-g-t 2/7] lib/igt_kms: refactor atomic properties load/commit Lionel Landwerlin
2016-03-23 12:13 ` Marius Vlad
2016-03-23 13:57 ` Lionel Landwerlin
2016-03-23 11:38 ` [PATCH i-g-t 3/7] lib/igt_kms: don't store rotation property id twice Lionel Landwerlin
2016-03-23 11:38 ` [PATCH i-g-t 4/7] lib/igt_kms: don't store background " Lionel Landwerlin
2016-03-23 11:38 ` [PATCH i-g-t 5/7] lib/igt_kms: don't store color management properties ids twice Lionel Landwerlin
2016-03-23 11:38 ` [PATCH i-g-t 6/7] lib/igt_kms: rename igt_crtc_set_background Lionel Landwerlin
2016-03-23 12:27 ` Daniel Vetter
2016-03-23 14:39 ` Lionel Landwerlin [this message]
2016-03-23 11:38 ` [PATCH i-g-t 7/7] lib/igt_kms: add some missing documentation Lionel Landwerlin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56F2AA9E.3090501@intel.com \
--to=lionel.g.landwerlin@intel.com \
--cc=daniel@ffwll.ch \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.